andygrove opened a new pull request, #5261: URL: https://github.com/apache/datafusion-comet/pull/5261
Backport of #5192 to `branch-1.0`. Cherry-picked cleanly from `2af9cec38b1f68550c3e13c1109f3c7502081e84` with no conflicts and no modifications — the diff is identical to the original PR. ## Which issue does this PR close? Closes #2838 and #5224 on `branch-1.0`. ## Rationale for this change Comet previously routed `GenerateExec` with `outer = true` back to Spark (`Incompatible`) because DataFusion's `UnnestExec` with `preserve_nulls = true` emits one null row for a NULL list but drops rows whose list is empty. Spark's `explode_outer` / `posexplode_outer` must emit one null row for *both* cases, so anything containing empty arrays fell back to JVM whole-stage codegen. ## What changes are included in this PR? Native: - New `ListEmptyToNullExpr` (`native/core/src/execution/expressions/list_empty_to_null.rs`) rewrites a `List<T>` to mark every empty row as null while preserving the original offsets, values, and column name. - `planner.rs` wraps the array child with `ListEmptyToNullExpr` when `explode.outer` is true, before positions are computed and before the projection feeds `UnnestExec`. `ListPositionsExpr` inherits the modified null bitmap so `pos` and `value` stay aligned for `posexplode_outer`. Serde: - `CometExplodeExec.getSupportLevel` no longer returns `Incompatible` for `op.outer`. Unsupported cases (maps, non-deterministic generators, multi-input generators, `COMET_EXEC_EXPLODE_ENABLED = false`) still fall back to Spark whole-stage codegen through the standard `Unsupported` path. Tests: - Un-ignored `explode_outer with empty array`, `explode_outer with nullable projected column`, `explode_outer with mixed null, empty, and non-empty arrays` in `CometGenerateExecSuite`. - Dropped the `WHERE id != 4` workaround and stale `allowIncompatible` `Config:` directive in `posexplode.sql`. - Added `sql-tests/expressions/array/explode.sql` covering `explode` / `explode_outer` (plus `LATERAL VIEW` and `LATERAL VIEW OUTER`) across every primitive element type, nested `array<array<int>>`, `array<struct>`, NULLs, literal arrays, empty tables, and an `expect_fallback` for `map` input. ## Are these changes tested? Yes — same tests as the original PR. Verified locally on `branch-1.0`: `cargo build` succeeds and all 18 `list_empty_to_null` / `list_positions` / related Rust unit tests pass. `branch-1.0` pins the same DataFusion version (54.1.0) as `main`, so no API adaptation was needed. ## Are there any user-facing changes? `explode_outer` and `posexplode_outer` now run natively without requiring `spark.comet.operator.GenerateExec.allowIncompatible = true`. No behavioral change for `explode` / `posexplode`. -- 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]
