What is the problem the feature request solves?
Plain posexplode evaluates its array argument twice per batch. posexplode_outer does not — it already has the fix — so this is an inconsistency between two adjacent branches of the same planner match rather than a missing feature.
QueryPlanSerde sends the generator's child as one expression, and create_plan's Explode arm puts it into the pre-explode projection twice when position is set: once wrapped in ListPositionsExpr to produce pos, and once as the array column that ExplodeExec unnests.
https://github.com/apache/datafusion-comet/blob/main/native/core/src/execution/planner.rs#L2121-L2126
ProjectionExec evaluates each expression in its list independently, so the child runs once per branch. When the child is an attribute that costs nothing, the second reference is an Arc clone. When it is a real expression it is real work, and when it is an expression Comet does not implement natively it is a JNI round-trip per batch, because codegen dispatch sends it back to the JVM as a JvmScalarUdf.
The outer variant avoids this. Comet has to wrap the array in ListEmptyToNullExpr there to get Spark's outer semantics, and rather than re-running that wrapper per branch it materializes it in a pre-projection under a reserved name, so the upper projection references a column:
https://github.com/apache/datafusion-comet/blob/main/native/core/src/execution/planner.rs#L2063-L2092
The comment above that block reasons about the wrapper only, and concludes that the non-outer cases reference the array once so need no pre-projection. That holds for explode and explode_outer. It does not hold for posexplode, where the raw child is the thing referenced twice.
Reproduction
SELECT posexplode(split(s, 'x')) FROM t against a one-column Parquet table, with spark.comet.explain.native.enabled=true. split is not native here, so it arrives as a JvmScalarUdf:
CometExplodeExec, elapsed_compute=205.00µs
ProjectionExec: expr=[list_positions(JvmScalarUdf(..., s@0)) as pos,
JvmScalarUdf(..., s@0)]
expr_0_eval_time=5.07ms, expr_1_eval_time=1.59ms
Both projection expressions pay for the same split. The same query as posexplode_outer evaluates it once, in the pre-projection, and the upper projection is free:
CometExplodeExec, elapsed_compute=2.50µs
ProjectionExec: expr=[list_positions(__comet_explode_outer_...@1) as pos,
__comet_explode_outer_...@1 as ...]
expr_0_eval_time=2.00µs, expr_1_eval_time=41ns
ProjectionExec: expr=[s@0 as s, list_empty_to_null(JvmScalarUdf(..., s@0)) as __comet_explode_outer_...]
expr_1_eval_time=4.15ms
4.15ms of work once, against 5.07ms + 1.59ms for the same query without outer.
This is invisible in the Spark plan, which shows one CometProject either way. It shows up as a CometProject under a CometExplode that is far slower than the explode above it — though note that projection is also where a non-native array expression legitimately runs, so a slow one there is not by itself evidence of this bug. The expr_N_eval_time metrics are what distinguish the two.
Describe the potential solution
Take the pre-projection whenever position is set, not only when outer is, wrapping with ListEmptyToNullExpr only in the outer case. The existing (true, true) branch is most of the code already; the match arms collapse to something like (outer, true) building the pre-projection over raw_child_expr or its wrapped form, and (_, false) unchanged.
Two things worth getting right:
- Skip the pre-projection when the child is already a
Column, which is the common posexplode(array_col) shape. Otherwise every such query gains a ProjectionExec for no benefit.
- The reserved output name is currently built as
format!("__comet_explode_outer_{}", child_field_name), which will read oddly once non-outer plans use it. Worth renaming while the code is being touched.
No behavior change is expected, so CometGenerateExecSuite should pass untouched. The measurable claim is that expr_1_eval_time on the upper projection drops to near zero and total time for posexplode over a non-trivial array expression approaches posexplode_outer over the same one.
Additional context
Found while looking at explode performance more broadly. Related but separate: #5667 optimizes the unnesting kernels inside ExplodeExec, which is the operator above this projection and does not touch how the array expression is fed to it. Issue #5210 tracks retiring ListEmptyToNullExpr in favor of upstream unnest_outer; if that lands first, the outer branch loses its wrapper but the double-reference in the position case remains, since it is about the raw child and not the wrapper.
What is the problem the feature request solves?
Plain
posexplodeevaluates its array argument twice per batch.posexplode_outerdoes not — it already has the fix — so this is an inconsistency between two adjacent branches of the same planner match rather than a missing feature.QueryPlanSerdesends the generator's child as one expression, andcreate_plan'sExplodearm puts it into the pre-explode projection twice whenpositionis set: once wrapped inListPositionsExprto producepos, and once as the array column thatExplodeExecunnests.https://github.com/apache/datafusion-comet/blob/main/native/core/src/execution/planner.rs#L2121-L2126
ProjectionExecevaluates each expression in its list independently, so the child runs once per branch. When the child is an attribute that costs nothing, the second reference is anArcclone. When it is a real expression it is real work, and when it is an expression Comet does not implement natively it is a JNI round-trip per batch, because codegen dispatch sends it back to the JVM as aJvmScalarUdf.The
outervariant avoids this. Comet has to wrap the array inListEmptyToNullExprthere to get Spark's outer semantics, and rather than re-running that wrapper per branch it materializes it in a pre-projection under a reserved name, so the upper projection references a column:https://github.com/apache/datafusion-comet/blob/main/native/core/src/execution/planner.rs#L2063-L2092
The comment above that block reasons about the wrapper only, and concludes that the non-outer cases reference the array once so need no pre-projection. That holds for
explodeandexplode_outer. It does not hold forposexplode, where the raw child is the thing referenced twice.Reproduction
SELECT posexplode(split(s, 'x')) FROM tagainst a one-column Parquet table, withspark.comet.explain.native.enabled=true.splitis not native here, so it arrives as aJvmScalarUdf:Both projection expressions pay for the same
split. The same query asposexplode_outerevaluates it once, in the pre-projection, and the upper projection is free:4.15ms of work once, against 5.07ms + 1.59ms for the same query without
outer.This is invisible in the Spark plan, which shows one
CometProjecteither way. It shows up as aCometProjectunder aCometExplodethat is far slower than the explode above it — though note that projection is also where a non-native array expression legitimately runs, so a slow one there is not by itself evidence of this bug. Theexpr_N_eval_timemetrics are what distinguish the two.Describe the potential solution
Take the pre-projection whenever
positionis set, not only whenouteris, wrapping withListEmptyToNullExpronly in the outer case. The existing(true, true)branch is most of the code already; the match arms collapse to something like(outer, true)building the pre-projection overraw_child_expror its wrapped form, and(_, false)unchanged.Two things worth getting right:
Column, which is the commonposexplode(array_col)shape. Otherwise every such query gains aProjectionExecfor no benefit.format!("__comet_explode_outer_{}", child_field_name), which will read oddly once non-outer plans use it. Worth renaming while the code is being touched.No behavior change is expected, so
CometGenerateExecSuiteshould pass untouched. The measurable claim is thatexpr_1_eval_timeon the upper projection drops to near zero and total time forposexplodeover a non-trivial array expression approachesposexplode_outerover the same one.Additional context
Found while looking at explode performance more broadly. Related but separate: #5667 optimizes the unnesting kernels inside
ExplodeExec, which is the operator above this projection and does not touch how the array expression is fed to it. Issue #5210 tracks retiringListEmptyToNullExprin favor of upstreamunnest_outer; if that lands first, the outer branch loses its wrapper but the double-reference in thepositioncase remains, since it is about the raw child and not the wrapper.