andygrove opened a new pull request, #6989: URL: https://github.com/apache/datafusion-comet/pull/6989
## Which issue does this PR close? Closes #6768. Part of #6385. ## Rationale for this change Spark 4.0.5, 4.1.4 and 4.2.1 normalize each flat `FLOAT` or `DOUBLE` element before `arrays_overlap` looks it up in its `java.util.HashSet` of boxed values (SPARK-59602), so `-0.0` and `0.0` are one value. Spark 4.2.0 gets the same answer by normalizing the arguments in the plan (SPARK-54918), and earlier releases keep the zeros apart. Comet's native `spark_arrays_overlap` keeps them apart on every version, so on those releases `arrays_overlap(array(d), array(-d))` over `d = 0.0` returns `false` where Spark returns `true`. The releases are still candidates (`v4.0.5-rc2`, `v4.1.4-rc2` and `v4.2.1-rc2`), and #6768 lists the fixture rows that fail against them. ## What changes are included in this PR? - **Native:** `SparkArraysOverlap` has a `normalize_floats` flag and is registered twice, as `spark_arrays_overlap` and `spark_arrays_overlap_normalized`. The flag changes only the flat `FLOAT` and `DOUBLE` arms, which key elements by `normalize_float` instead of `canonicalize_nan`, so the zeros share a key without a copy of the input. Both variants treat every NaN as one value, and both compare nested elements in Spark's SQL ordering, as before. - **Serde:** `CometArraysOverlap.nativeFunction` picks the normalized variant wherever `ArraySetSupport.normalizesFloats` holds. That is the patch-level version check `array_distinct` and `array_union` already use: 4.0.5+, 4.1.4+ and 4.2+. - **Docs:** the compatibility guide has an `arrays_overlap` section. It notes that the check reads the version number, so on a vendor build that carries SPARK-59602 under an older version, Comet still keeps the zeros apart. The contributor guide's list of rules that changed in a release and the array audit cover it too. Nothing changes on a Spark release that has shipped. 4.2.0 now runs the normalized variant over input the plan has already normalized, and the other releases keep the current kernel. ## How are these changes tested? - **Rust:** the two flat float tests check both variants, over short lists and over lists long enough to take the hash probe. Their rows show that only the zeros merge and that other values keep their sign. A UDF-level test checks each variant's name and that the flag reaches the kernel for `List` and `LargeList` columns and for scalars. Each of seven mutants fails at least one of these tests: the flag ignored in the kernel, a key that also drops the sign of other values, `false` passed at each of the four call sites, and `normalized()` building the old variant. All 1,213 `spark-expr` tests pass, and `cargo fmt` and clippy (`--all-targets -D warnings`, stable and nightly) are clean. - **Scala:** a test checks `nativeFunction` on both sides of each version boundary, and that `convert` asks for the variant of the running version. On Spark 4.2 that check fails with `convert` hardcoded to `spark_arrays_overlap`. - **Spark 4.1 and 4.2:** `CometArrayExpressionSuite`, `CometFloatSemanticsSuite` and the `expressions/array/` fixtures pass, 574 tests on each. On 4.2, every native `arrays_overlap` among them runs the normalized variant. On 3.4 the `arrays_overlap` tests pass too. - **Not covered:** none of the Spark versions Comet builds against can show the merge, because 4.2.0 normalizes in the plan before the kernel runs. I haven't run the suites against the release candidates for this change. -- 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]
