andygrove opened a new issue, #5572: URL: https://github.com/apache/datafusion-comet/issues/5572
## What / Why Comet has a JVM codegen dispatcher (`CometScalaUDF.emitJvmCodegenDispatch`, `spark/src/main/scala/org/apache/comet/serde/CometScalaUDF.scala:70`) that compiles a Spark expression's own `doGenCode` output into a per-batch kernel reading and writing Arrow vectors directly. When a serde has no native path for some input, routing through the dispatcher keeps the whole operator inside Comet and still matches Spark byte for byte, instead of failing the enclosing projection back to Spark. A serde opts in by mixing in `CodegenDispatchFallback` (`spark/src/main/scala/org/apache/comet/serde/CometExpressionSerde.scala:125`). `QueryPlanSerde` then tries the dispatcher for that serde's `Unsupported` and non-opt-in `Incompatible` results (`QueryPlanSerde.scala:941` and `:968`) before giving up. Adoption is uneven. I swept all 261 expression serdes under `spark/src/main/scala/org/apache/comet/serde/` plus the version shims. 98 reach the dispatcher; 63 decline at least one case without one. This EPIC collects the cases where the mixin is applicable and worth adding, and — just as importantly — records the cases where it is *not* applicable, so they aren't re-litigated. Several of the gaps are plain asymmetries against an already-dispatched sibling: `bround` dispatches but `round` on a double falls back; `to_unix_timestamp` dispatches but `unix_timestamp` on a string falls back; `to_json`, `from_csv` and `schema_of_csv` dispatch but `to_csv` never runs in Comet by default at all. ## What gates a dispatch `CometBatchKernelCodegen.canHandle` (`spark/src/main/scala/org/apache/comet/codegen/CometBatchKernelCodegen.scala:119`) rejects `AggregateFunction`, `Generator`, `Unevaluable`, and any bound reference or output type outside `isSupportedDataType` (`:85`) — notably `NullType`, `ObjectType` and Variant. Everything else is admitted, including `CodegenFallback`, nondeterministic and stateful expressions, `HigherOrderFunction`, and subquery expressions. Every item below was checked against that gate. ## Prerequisites <!--PREREQS--> ## High value <!--TIER1--> ## Worthwhile <!--TIER2--> ## Considered and rejected — structurally impossible These are recorded so the sweep doesn't get repeated on them: - **All 25 aggregate serdes.** Blocked twice over: `canHandle` rejects `AggregateFunction` outright, and `CodegenDispatchFallback`'s self-type is `CometExpressionSerde[_]`, so it cannot be mixed into `CometAggregateExpressionSerde` at all. This covers `percentile` with an array of percentages, `approx_percentile` on non-numeric input, `collect_list` / `collect_set`, `bloom_filter_agg`, `avg` / `sum` on intervals, and the `bit_and` / `bit_or` / `bit_xor` family. - **`CometHours` / `CometDays`** (`serde/datetime.scala:813`, `:850`). Verified against the Spark 4.0.1 bytecode: `Hours` and `Days` extend `PartitionTransformExpression`, which implements `Unevaluable`, so `canHandle` rejects them by construction. - **`CometLiteral`, `CometAttributeReference`, `CometKnownFloatingPointNormalized`.** What they decline is exactly the set of types that cannot cross the Arrow FFI boundary — the same set `isSupportedDataType` rejects. The dispatcher would decline them again. - **Null-element array cases.** The "null elements fall back" half of the `array_position` / `flatten` / `shuffle` notes in the compatibility guide is not dispatchable: `NullType` is absent from `isSupportedDataType`. Only the binary/struct half of those notes is actionable. - **`CometSortOrder`.** Sort keys are ordering specifications consumed by the native Sort and Window operators, not value expressions, so there is nothing to dispatch. - **`CometScalarFunction`'s ANSI guard** (`serde/CometScalarFunction.scala:30`). A developer mis-wiring check, not a runtime input case. ## Considered and deferred — marginal Not filed, but recorded so the reasoning survives: negative-scale decimal in `CometCeil` / `CometFloor` / `CometRound` (only reachable with `spark.sql.legacy.allowNegativeScaleOfDecimal=true`); `CometToPrettyString` (only on the `df.show()` path); non-literal seeds in `CometRandStr` / `CometRand` / `CometRandn`; the deliberate all-foldable declines in `CometConcatWs` and `CometArrayPosition`, which are a handoff to `ConstantFolding` and should stay; and the unreachable sanity checks in `CometDivide`, `CometCheckOverflow`, `CometMakeDecimal`, `CometSize`, `CometPreciseTimestampConversion`, `CometLeft` and `CometRight`. ## Suggested sequencing 1. The two prerequisites. The closure-serialize guard in particular blocks the `StaticInvoke` catch-all. 2. The high-value items. Each is a small change and each closes a documented asymmetry against an already-dispatched sibling, which makes them easy to review. 3. The rest, in any order — they are independent and mostly good first issues. ## Verification note This sweep is static analysis of the serde definitions, the dispatcher's `canHandle` gate, and the generated `docs/source/user-guide/latest/expressions.md`. Nothing here has been reproduced against a running cluster yet, so each issue should confirm the fallback with a test before the fix lands. -- 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]
