grorge123 commented on PR #5526: URL: https://github.com/apache/datafusion-comet/pull/5526#issuecomment-5645951194
Thanks, all three addressed in one commit on top, rebased onto main (`3810936b4`). **Dispatcher enrollment.** `CometArrayRepeat`, `CometArrayUnion`, `CometCreateArray`, `CometIf`, `CometCaseWhen` and `CometCoalesce` now mix in `CodegenDispatchFallback`, so their `Unsupported` cases run through the JVM codegen dispatcher and the plan stays fully native. `CometSlice` needs no gate after the rebase: #5766's `convert` no longer serializes a return type, so the mismatch it guarded against cannot happen and `slice(map_entries(map(k, NULL)), 1, 1)` runs natively. On `array_repeat`, your example made me look closer: `array_repeat(array(c), 2)` runs natively on main because native `make_array` emits a nullable item whatever Spark's `containsNull` says; what fails is an input whose native item is really non-nullable (`map_entries`, "ListArray expected data type List(non-null Struct(..)) got List(Struct(..))"). The gate now exempts a `CreateArray` child, so `array(c)` stays on the kernel and `map_entries(..)` goes through the dispatcher. Enrolling `coalesce` exposed a disp atcher bug: a top-level `struct<.., x: null>` output failed to allocate (`StructVector`'s constructor builds a `NullableStructWriter` over the field's children, which has no arm for a Null child); the output vector now adds its children after construction, with a unit test. One boundary: the dispatcher accepts NullType as an output but not as an input column, so a gated shape whose NullType side arrives as an upstream column still falls back. The SQL file tests that asserted a fallback now assert the native plan, and `CometNullTypeCompositionSuite` checks per gate that the serde is enrolled, the reason is published, and the query keeps its `CometProjectExec`. **Compatibility pages.** Every serde whose support level moved publishes the reason through `getUnsupportedReasons`, following `HashUtils`; the runtime note starts with the published text and the suite asserts that, so the two cannot drift apart. **Suite size.** It was 37 tests in 10 min 49 s on an i7-12700K, so likely two to three times that on a runner, and the consumer sweeps were the bulk. Each sweep now runs only under the profiles that can change its outcome (a null guard is per row, so batching cannot affect the nullable sweep; a stateful producer only diverges across a batch boundary, so the kernel choice cannot affect that one; a non-deterministic child leaves every kernel, so neither reaches the last), the columnar-to-row-only physical profile is folded into the sort-writer one, and Spark's reference rows are cached across profiles with the same row count. That is 28 tests in about 8 min on the same machine, with the operator and nesting sweeps still under every shuffle path and both batch sizes; nothing removed had ever caught a failure the kept combinations did not. Rebase notes: `CometElementAt` takes main's #5544 fix, the `NullArray` special case in `GetStructField` moved into #5769's shared `child_with_parent_nulls`, and the two duplicate-field-name checks are folded into one with main's reason wording. Assisted-by: Claude Code (claude-fable-5) -- 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]
