rich7420 opened a new pull request, #6829: URL: https://github.com/apache/datafusion-comet/pull/6829
## Which issue does this PR close? Closes #6519. ## Rationale for this change Spark's primitive double-array sort places every NaN last, while Rust's total order places negative-sign NaNs before all finite values. After multiple 50,000-value head-buffer flushes, native `percentile_approx` can therefore merge samples out of order and return incorrect percentiles, including a finite maximum instead of NaN. Negating a NaN after reading Parquet reproduces this on either architecture. ## What changes are included in this PR? - Keep the existing total-order sort, then rotate its leading negative-NaN block to the end. A first-value check skips the boundary search for ordinary buffers. The correction uses no extra buffer, preserves NaN bits and Java's signed-zero ordering, and leaves compression, merge and query arithmetic unchanged. - Add kernel regressions for six NaN sign/payload representations across six head-buffer flushes, serialized partial-summary merges, infinities and signed zero. - Add a SQL fixture that generates negative NaNs after the Parquet scan and checks native Double/Float inputs, scalar/array outputs and grouped partial/final aggregation against Spark. ## How are these changes tested? Rebased onto main `6960a48ae8d302ed6b836f3213367ebecebb9515` at head `c3ee93fe122b6c5c8259e1eaaf48ba9f5e285c88`. The two changed files and the feature patch are identical to the previously validated feature commit `65327af4b`. Current-source local checks: - Compile the dependency-free production quantile module directly with `rustc --test -Copt-level=2`: all 15 tests pass, with no failures or ignored tests. - Negative control: the same two NaN regressions fail against main's original production kernel and reproduce the issue's incorrect percentile ranks and finite maximum. - Rustfmt and Git whitespace checks pass. Historical integration evidence at `65327af4b`: 169 native aggregate tests passed; the existing and new approximate-percentile SQL fixtures passed on Spark 3.5.9 and 4.1.3 (20 queries per profile, requiring native execution and exact Spark results). The new SQL fixture failed with the original kernel. Release JNI provenance, expression-crate Clippy over all targets and Spotless were verified there, and [fork CI](https://github.com/rich7420/datafusion-comet/actions/runs/37020033188/attempts/2) passed. These are pre-rebase results, not execution evidence for the new integration head. Historical component timings on the unchanged kernel showed finite-control median differences of +0.5% to +3.0% and -1.6% for many small groups. Dense negative NaNs took about 6% longer than the baseline canonical-positive-NaN control, which does not require block rotation. These are component measurements with different NaN bits, not a Spark SQL speedup claim; timings were not rerun during this rebase. Current-head full native/JNI and Spark integration checks are pending hosted CI. [Fork PR #45](https://github.com/rich7420/datafusion-comet/pull/45) requests all Comet Spark profiles and the Spark 4.1 SQL suites using `run-all-spark-profiles` and `run-spark-4.1-tests`. This PR remains Draft until that verdict. -- 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]
