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

   ## Which issue does this PR close?
   
   Partially addresses #5401. The issue remains open for native signed-zero tie 
parity; this PR fixes strict-mode evaluation.
   
   ## Why are the changes needed?
   
   Spark treats `+0.0` and `-0.0` as equal when finding array extrema and 
retains the first equal element. The native implementation orders `-0.0` before 
`+0.0`, so a tied minimum or maximum can have the wrong sign. `CometArrayMin` 
and `CometArrayMax` currently classify these cases as compatible even when 
`spark.comet.exec.strictFloatingPoint=true`.
   
   ## What changes were proposed in this PR?
   
   - Mark extrema whose result types contain float or double as incompatible in 
strict floating-point mode, using one shared support-level check and reason.
   - Use the existing `CodegenDispatchFallback` path to evaluate those cases 
with Spark's expression codegen inside Comet. Fall back to Spark when that 
dispatcher is unavailable.
   - Preserve non-strict native behavior, non-floating native execution, and 
the explicit `allowIncompatible` native opt-in.
   - Add exact tests for both zero orders and widths, column and literal 
inputs, nested arrays, null/empty inputs, dictionary encoding, 
disabled-dispatcher fallback, and explicit native opt-in.
   - Link the ignored default-mode native reproductions to #5401 and document 
the remaining difference.
   - Scope generated compatibility wording to the listed incompatible cases, so 
configuration-dependent expressions do not incorrectly claim unconditional JVM 
evaluation by default.
   
   This changes Comet's Spark-compatibility routing, not DataFusion or Arrow 
comparison semantics. No native code or dependency changes are included.
   
   ## How was this PR tested?
   
   - Built the unchanged native code with `cargo build --locked --offline` and 
JDK 17.
   - Ran focused Comet SQL tests on Spark 3.4.3 and 4.1.3: 12 tests passed on 
each version.
   - Verified the new strict min/max fixtures fail on the unpatched Spark 3.4.3 
Comet runtime: all four dictionary variants fail on the expected zero-sign 
mismatches, with native execution confirmed in the plans.
   - Checked the Spark extrema implementations for 3.4.3, 3.5.8, 4.0.1, and 
4.1.1, plus the cross-version expression API.
   - Ran the actual documentation generator into a scratch tree and verified 
both extrema sections describe conditional routing and reference #5401.
   - Scala compilation with Scalastyle enabled, Spotless, Prettier, and `git 
diff --check` passed.
   
   Each selector was run separately:
   
   ```sh
   ./mvnw test -Pspark-4.1 -Dtest=none -Dscalastyle.skip=true 
-Dsuites="org.apache.comet.CometSqlFileTestSuite array_min"
   ./mvnw test -Pspark-4.1 -Dtest=none -Dscalastyle.skip=true 
-Dsuites="org.apache.comet.CometSqlFileTestSuite array_max"
   ./mvnw test -Pspark-4.1 -Dtest=none -Dscalastyle.skip=true 
-Dsuites="org.apache.comet.CometSqlFileTestSuite array_extrema_strict_fp"
   ```
   
   Repeated with `-Pspark-3.4`, using a clean reactor build when switching 
profiles. Local runs used dependency-cache and native-library-directory 
overrides. The full test suite was not run.
   


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