You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
CometCastSuite was written when a cast was either implemented natively or fell back to Spark
for the whole plan. CometCast now mixes in CodegenDispatchFallback, so an Unsupported
support level no longer means the plan leaves Comet -- the JVM codegen dispatcher runs Spark's
own generated code for the cast inside the Comet pipeline, and the results are
Spark-compatible by construction.
Several assumptions baked into the suite no longer hold:
all valid cast combinations covered conflates "unsupported" with "broken". The
assertion requires that any pair where CometCast.isSupported is not Compatible must have
an ignored test, on the grounds that the test would otherwise fail. For a
codegen-dispatched cast the test would now pass, so the assertion forces us to ignore
working coverage. See the cast BooleanType to DecimalType(10,2) case added in feat: remove native cast from boolean to decimal #5185, where the test is ignored purely to
satisfy this rule while the DecimalType(14,4) and DecimalType(30,0) variants stay active
and pass.
Existing ignored tests may be stale. Many are ignored with comments about wrong results
or Arrow errors from the native path (for example the cast TimestampType to ByteType family).
Those casts are now dispatched rather than executed natively, so some of them likely produce
correct results today. Nobody has re-checked them since the dispatcher landed.
castTest's hasIncompatibleType flag is ambiguous. It currently means "do not assert
native execution". With dispatch, a cast can be non-native in the sense of having no Rust
kernel while still keeping the plan fully inside Comet, and the flag cannot express that
distinction.
Describe the potential solution
Roughly, in increasing order of effort:
Teach all valid cast combinations covered about the three outcomes we now care about --
native, codegen-dispatched, and genuine Spark fallback -- instead of the current
compatible/ignored binary, so a dispatched cast can keep an active test.
Audit the ignored tests and un-ignore the ones that now pass through the dispatcher, updating
or deleting the stale comments and issue links.
Split hasIncompatibleType into distinct expectations for "runs natively", "runs in Comet via
the dispatcher", and "falls back to Spark".
What is the problem the feature request solves?
CometCastSuitewas written when a cast was either implemented natively or fell back to Sparkfor the whole plan.
CometCastnow mixes inCodegenDispatchFallback, so anUnsupportedsupport level no longer means the plan leaves Comet -- the JVM codegen dispatcher runs Spark's
own generated code for the cast inside the Comet pipeline, and the results are
Spark-compatible by construction.
Several assumptions baked into the suite no longer hold:
all valid cast combinations coveredconflates "unsupported" with "broken". Theassertion requires that any pair where
CometCast.isSupportedis notCompatiblemust havean
ignored test, on the grounds that the test would otherwise fail. For acodegen-dispatched cast the test would now pass, so the assertion forces us to ignore
working coverage. See the
cast BooleanType to DecimalType(10,2)case added infeat: remove native cast from boolean to decimal #5185, where the test is ignored purely to
satisfy this rule while the
DecimalType(14,4)andDecimalType(30,0)variants stay activeand pass.
Existing
ignored tests may be stale. Many are ignored with comments about wrong resultsor Arrow errors from the native path (for example the
cast TimestampType to ByteTypefamily).Those casts are now dispatched rather than executed natively, so some of them likely produce
correct results today. Nobody has re-checked them since the dispatcher landed.
castTest'shasIncompatibleTypeflag is ambiguous. It currently means "do not assertnative execution". With dispatch, a cast can be non-native in the sense of having no Rust
kernel while still keeping the plan fully inside Comet, and the flag cannot express that
distinction.
Describe the potential solution
Roughly, in increasing order of effort:
all valid cast combinations coveredabout the three outcomes we now care about --native, codegen-dispatched, and genuine Spark fallback -- instead of the current
compatible/ignored binary, so a dispatched cast can keep an active test.
or deleting the stale comments and issue links.
hasIncompatibleTypeinto distinct expectations for "runs natively", "runs in Comet viathe dispatcher", and "falls back to Spark".
Additional context
Context in #5185.