Repository navigation
fix: [branch-1.1] treat -0.0 and 0.0 as one value in arrays_overlap on Spark 4.0.5, 4.1.4 and 4.2.1 - #7265
Open
snmvaughan wants to merge 2 commits into
Open
fix: [branch-1.1] treat -0.0 and 0.0 as one value in arrays_overlap on Spark 4.0.5, 4.1.4 and 4.2.1#7265snmvaughan wants to merge 2 commits into
snmvaughan wants to merge 2 commits into
Conversation
…, 4.1.4 and 4.2.1 (apache#6989) * fix: treat -0.0 and 0.0 as one value in arrays_overlap on Spark 4.0.5, 4.1.4 and 4.2.1 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 hash set of boxed values (SPARK-59602), so -0.0 and 0.0 are one value, as they already were on 4.2.0, which normalizes the arguments in the plan (SPARK-54918). The native kernel kept the zeros apart on every version, so on those releases it returned false where Spark returns true. Register a second variant, spark_arrays_overlap_normalized, whose flat float arms key elements by normalize_float instead of canonicalize_nan, and have CometArraysOverlap pick it wherever ArraySetSupport.normalizesFloats holds. Earlier releases keep the existing kernel. Both variants treat every NaN as one value and compare nested elements in Spark's SQL ordering. Closes apache#6768. * review: name the variant in arrays_overlap errors and trim the version table Use self.name() in the unsupported-type errors so spark_arrays_overlap_normalized reports itself, and test it on the array and scalar paths. Reduce the native docs to the two equality policies and point at CometArraysOverlap.nativeFunction for the version mapping. Keep one Spark version on each side of the boundary in the routing test and the convert assertion. Adapted for branch-1.1: - branch-1.1 has no ArraySetSupport.normalizesFloats (apache#6563), so this adds it next to normalizesArgumentsInPlan, for arrays_overlap only. array_distinct and array_union keep branch-1.1's 4.2.0-only check. - arrays_overlap.rs takes normalize_float from math_funcs::internal and keeps branch-1.1's own NaN keying, since branch-1.1 predates the float_semantics module (apache#6400). - The new test pins the version boundaries itself, since branch-1.1 doesn't have the test that does on main. - The user-guide floating-point page gets the arrays_overlap section only. The contributor-guide pages are dropped, since that guide isn't published from release branches. (cherry picked from commit 3565f2f)
github-actions
Bot
added
bug
snmvaughan
changed the title
snmvaughan
changed the title
andygrove
marked this pull request as ready for review
October 27, 2026 22:48
Member
|
@snmvaughan could you fix conflicts? |
Resolves the import conflict with apache#7266 in CometArrayExpressionSuite: the serde import takes both sides' names.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Backport of #6989 (squash commit 3565f2f) to
branch-1.1, for #6768.Rationale for this change
branch-1.1has the bug #6989 fixes onmain. Spark 4.0.5, 4.1.4 and 4.2.1 normalize each flatFLOATorDOUBLEelement beforearrays_overlaplooks it up (SPARK-59602), so-0.0and0.0are one value, and Spark 4.2.0 gets the same answer by normalizing the arguments in the plan (SPARK-54918). Onbranch-1.1,CometArraysOverlaptreats floating-point elements as compatible and runs the nativespark_arrays_overlap, which keeps the zeros apart. So on those releasesarrays_overlap(array(0.0), array(-0.0))returnsfalsewhere Spark returnstrue, by default. See #6768.What changes are included in this PR?
git cherry-pick -xof 3565f2f, adapted becausebranch-1.1predates two changes #6989 builds on:branch-1.1has noArraySetSupport.normalizesFloats(feat: normalize floats in native array_distinct and array_union #6563), so this adds it next tonormalizesArgumentsInPlan, forarrays_overlaponly.array_distinctandarray_unionkeepbranch-1.1's 4.2.0-only check.arrays_overlap.rstakesnormalize_floatfrommath_funcs::internaland keepsbranch-1.1's own NaN keying, sincebranch-1.1predates thefloat_semanticsmodule (refactor: move native -0.0 and NaN handling into one float_semantics module #6400). Otherwise the native change is the same:SparkArraysOverlaphas anormalize_floatsflag and is registered as bothspark_arrays_overlapandspark_arrays_overlap_normalized, andCometArraysOverlap.nativeFunctionpicks the variant by the Spark version.branch-1.1doesn't have the test that does onmain.arrays_overlapsection only. The contributor-guide pages are dropped, since that guide isn't published from release branches.Backports to other branches: none for
branch-1.2, which already has #6989.How are these changes tested?
The native tests come with the cherry-pick, and the flat
FLOATandDOUBLEtests check both variants. On this branch, all 978datafusion-comet-spark-exprunit tests pass,cargo fmt --check,cargo clippy --all-targets --workspace -- -D warningsand spotless are clean, and the Scala code compiles. I haven't run the Scala suites locally.arrays_overlap native variant by Spark versioncovers the version boundaries and the variantconvertasks for.