IN LIST: treat signed zeros as equal - #25186
Open
geoffreyclaude wants to merge 1 commit into
Open
geoffreyclaude wants to merge 1 commit into
geoffreyclaude wants to merge 1 commit into
Conversation
github-actions
Bot
added
physical-expr
geoffreyclaude
force-pushed
the
codex/in-list-signed-zero
branch
5 times, most recently
from
September 11, 2026 13:58
12098fc to
474474f
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #25186 +/- ##
========================================
Coverage 81.91% 81.91%
========================================
Files 1134 1134
Lines 425637 425900 +263
Branches 425637 425900 +263
========================================
+ Hits 348654 348881 +227
- Misses 56302 56316 +14
- Partials 20681 20703 +22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
geoffreyclaude
force-pushed
the
codex/in-list-signed-zero
branch
2 times, most recently
from
September 11, 2026 14:36
e0f44e0 to
aa5a037
Compare
geoffreyclaude
commented
Sep 11, 2026
| let normalized: Float32Array = | ||
| arr.unary(|v| if v.to_bits() << 1 == 0 { 0.0_f32 } else { v }); | ||
| let normalized: Float32Array = arr.unary(|v| { | ||
| if v.to_bits() == NEG_ZERO_F32_BITS { |
Contributor
Author
There was a problem hiding this comment.
Only NEG_ZERO_F32_BITS needs a change, so compare directly against it
geoffreyclaude
commented
Sep 11, 2026
| let normalized: Float64Array = | ||
| arr.unary(|v| if v.to_bits() << 1 == 0 { 0.0_f64 } else { v }); | ||
| let normalized: Float64Array = arr.unary(|v| { | ||
| if v.to_bits() == NEG_ZERO_F64_BITS { |
Contributor
Author
There was a problem hiding this comment.
Only NEG_ZERO_F64_BITS needs a change, so compare directly against it
geoffreyclaude
force-pushed
the
codex/in-list-signed-zero
branch
from
September 11, 2026 15:02
aa5a037 to
eaee11e
Compare
geoffreyclaude
marked this pull request as ready for review
September 14, 2026 11:39
geoffreyclaude
force-pushed
the
codex/in-list-signed-zero
branch
from
September 14, 2026 11:40
eaee11e to
8713a66
Compare
Contributor
Author
|
run benchmark in_list_strategy |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing codex/in-list-signed-zero (8713a66) to 7b00b63 (merge-base) diff Run configurationrun benchmark in_list_strategyResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing codex/in-list-signed-zero (8713a66) to 7b00b63 (merge-base) diff Run configurationrun benchmark in_list_strategyCPU Details (lscpu)Details
Resource Usagein_list_strategy — base (merge-base)
in_list_strategy — branch
File an issue against this benchmark runner |
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
3 participants
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?
None
Rationale for this change
DataFusion equality treats
+0.0and-0.0as equal, but the optimizedINfilters compare or hash floating-point bit patterns. Because the two zeros have different encodings, anINpredicate could therefore return a different result depending on whether the optimizer retained it or rewrote it into equality comparisons.This PR makes top-level
Float16,Float32, andFloat64INpredicates follow the same signed-zero equality as ordinary comparisons. Static filters materialize both zero encodings once while building the list, keeping lookup on the row path branch-free. Non-static lists normalize evaluated floating operands once per batch. Distinct NaN payloads remain distinct, preserving the existing bit-equality behavior for NaNs.What changes are included in this PR?
Float16bitmap andFloat32/Float64hash-set filters.IN, andNOT IN.Are these changes tested?
The SQL logic tests cover both signed-zero directions for
Float16,Float32, andFloat64; comparison-rewritten, static-filter, and non-static-list paths; dictionary-encodedFloat64; and bothINandNOT IN. The added cases fail on the parent commit and pass with this fix.Focused validation:
cargo test -p datafusion-common normalize_float_zerocargo test -p datafusion-physical-expr in_list(88 tests)cargo test -p datafusion-sqllogictest --test sqllogictests -- negative_zeroAre there any user-facing changes?
Yes. Floating-point
INandNOT INnow treat+0.0and-0.0as equal, consistently with ordinary DataFusion equality. There are no public API changes.