[PR] perf: [branch-1.0] evaluate posexplode array expressions once per batch (#5737) [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove opened a new pull request, #5820:
URL: https://github.com/apache/datafusion-comet/pull/5820

   Backport of #5737 to `branch-1.0`.
   
   Cherry-picked from `cbe3b39035a5918fc65496ebfe08bcf8e7adfce2`. The planner change is
   byte-identical to the original PR; only the new Rust test needed adaptation, described under
   "What changes are included" below.
   
   Note that this is a performance enhancement rather than a bug fix, unlike the other backports on
   this branch so far. Including it in 1.0.1 is a release-management call, not something the change
   itself forces.
   
   ## Which issue does this PR close?
   
   Closes #5676 on `branch-1.0`.
   
   ## Rationale for this change
   
   Plain `posexplode` places its array expression in both the positions expression and the values
   projection, evaluating it twice per batch. `posexplode_outer` already materializes its wrapped
   array once. For a computed array, the duplicate evaluation can repeat native work or a JVM
   codegen-dispatch call.
   
   ## What changes are included in this PR?
   
   The planner change is the original one: apply the outer wrapper first, then materialize the array
   when positions are requested and the expression is not already a `Column`, so both positions and
   values reference the resulting column. Plain array-column inputs keep their existing single
   projection, and non-positional explode variants are unchanged. The intermediate column prefix is
   renamed to `__comet_explode_`.
   
   Two adaptations were needed for `branch-1.0`, both confined to the new Rust test:
   
   - Added the `AtomicUsize`/`Ordering`, `ArrayRef` and `MemorySourceConfig` imports the test needs.
     On `main` those were already in the test module, brought in by tests added after `branch-1.0`
     was cut.
   - Dropped `element_field_id: None` from the test's `ListInfo` literal. That proto field does not
     exist on `branch-1.0`.
   
   `branch-1.0` pins the same DataFusion version (54.1.0) as `main`, so no API adaptation was needed.
   
   ## How are these changes tested?
   
   Same tests as the original PR, verified locally on `branch-1.0`:
   
   - `cargo test -p datafusion-comet` passes 156 tests, including the new
     `explode_evaluates_array_once_per_batch` planner regression.
   - The regression was confirmed to fail on `branch-1.0` with the planner change reverted and the
     test kept, reproducing the count from the original PR: plain positional explode over a computed
     array evaluates the function 4 times for two batches instead of 2. So the duplicate evaluation
     is present on `branch-1.0` and this backport is what removes it.
   - `CometGenerateExecSuite` passes all 38 tests on Spark 4.1.3 / Java 17, including the new
     `posexplode with a computed array from Parquet`.
   - `CometSqlFileTestSuite` `expressions/array/posexplode.sql` passes.
   - `cargo clippy --all-targets --workspace -- -D warnings` and `cargo fmt --all -- --check` are
     clean, and Spotless and Scalastyle pass as part of the Maven run.
   - `test-compile -Pspark-3.4 -Pscala-2.12` succeeds, so the new Scala test cross-compiles on the
     oldest supported Spark/Scala combination.
   
   ## Are there any user-facing changes?
   
   No. Results are unchanged; a computed array passed to `posexplode` is now evaluated once per batch
   instead of twice.
   


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] perf: [branch-1.0] evaluate posexplode array expressions once per batch (#5737) [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove merged PR #5820:
URL: https://github.com/apache/datafusion-comet/pull/5820


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org