andygrove opened a new issue, #5186:
URL: https://github.com/apache/datafusion-comet/issues/5186
## What is the problem the feature request solves?
`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:
1. **`all valid cast combinations covered` conflates "unsupported" with
"broken".** The
assertion requires that any pair where `CometCast.isSupported` is not
`Compatible` must have
an `ignore`d 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
https://github.com/apache/datafusion-comet/pull/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.
2. **Existing `ignore`d 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.
3. **`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".
## Additional context
Context in https://github.com/apache/datafusion-comet/pull/5185.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
