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

   ## Which issue does this PR close?
   
   Closes #5544.
   
   This is a draft follow-up to #5452 and leaves that PR unchanged, following 
the agreement to address the remaining cases separately. It depends on #5452: 
the current main-based diff temporarily includes that PR's unmerged changes. 
The intended follow-up is these [three fix 
commits](https://github.com/sunchao/arrow-datafusion-comet/compare/67fbae59727d108dda1a875e5f00c33a545855e4...732e7acfc051a98ca4222208ebc68dbd21451193).
 They need to be rebased onto main after #5452 merges and validated again 
before this draft is marked ready.
   
   ## Rationale for this change
   
   The remaining cases in #5544 can return incorrect stateful lookup or 
collation results, evaluate a throwing key for a null collection, or fail 
because a constructed array's nested type differs from its declared result 
type. They require coverage beyond the existing SQL fixtures, which disable 
constant folding and normally run without ANSI mode.
   
   ## What changes are included in this PR?
   
   - Evaluate native map/array lookup inputs once, from left to right, and 
evaluate keys or indices only for non-null collections. This removes the Scala 
ANSI guard that duplicated stateful operands and covers bracket lookup and 
`try_element_at` as well as ordinary `element_at`.
   - Send collated `array_contains` comparisons through Spark codegen, 
including nested arrays/structs and mixed collated-string/floating-point 
values. `allowIncompatible=true` does not bypass the collation safeguard.
   - Keep the common nullable child type needed during array construction, then 
restore the declared nested element type before downstream `slice` or 
conditional expressions consume it. Retain `ArrayInsert`'s separate result-type 
normalization.
   
   The fixes are separate commits (`f6057825c`, `6ce68f786`, and `732e7acfc`). 
The branch also merges main at `98cd8c967`, preserving its unconditional 
aggregate-nullability normalization when resolving the planner conflict.
   
   ## How are these changes tested?
   
   Added native and Spark regression coverage for null/empty/scalar 
collections, selected stateful evaluation across batches, throwing keys, folded 
map literals, nested collations, and all-true/all-false/mixed conditional 
batches. Stateful Spark fixtures use one small ordered Parquet input so both 
rows exercise the same batch.
   
   Local validation on the combined branch, using Spark 4.1.3, JDK 17, and the 
rebuilt native development library:
   
   - 23 native tests: 12 lookup tests, 3 aggregate-nullability tests, and 8 
remote-shuffle schema tests.
   - 214 tests across the complete Array/Map expression suites and both TPC-DS 
plan-stability suites.
   - All 14 original reproductions and controls, including the seven previously 
failing variants.
   - All 12 changed SQL fixture runs, 5 nested-cast matrix tests, and 2 cache 
complex-type round trips.
   - Rust formatting, workspace Clippy with warnings denied, Maven Spotless, 
suite registration, and whitespace checks pass.
   
   Native code was built before the root-reactor JVM tests; the tests used 
current compiled sources without serializer overlays. This is 270 focused local 
checks, not a full-project, release-profile, cross-Spark-version, or live 
Celeborn cluster run. Hosted CI is checked separately.
   


-- 
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]

Reply via email to