Repository navigation
fix: treat -0.0 and 0.0 as equal in short float IN lists - #27122
Open
jonasHanhan wants to merge 1 commit into
Open
jonasHanhan wants to merge 1 commit into
jonasHanhan wants to merge 1 commit into
Conversation
Short Float32/Float64 IN lists are kept as membership expressions, which compare floats by bit pattern, so -0.0 did not match 0.0. Fall back to the equality chain when the list contains a zero literal.
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?
Rationale for this change
Since #25187, short
Float32/Float64literal IN lists are no longer rewritten into=/<>chains and stay as membership expressions. The membership filters compare floats by bit pattern, so-0.0 IN (0.0, 5.0)returns false even though-0.0 = 0.0is true. That's a behavior change from 55.x.What changes are included in this PR?
ShortenInListSimplifierstill keeps short float lists as IN lists, except when one of the literals is0.0or-0.0. In that case it falls back to the comparison chain like before, so IN agrees with=. Lists without a zero keep the faster path.I went with the narrow fix rather than changing the filters to normalize zeros, since the bit-equality behavior there is covered by existing tests and also affects longer lists (the case mentioned at the end of the issue, which already behaves this way in 55). Happy to look at that separately if you'd prefer it handled in the filters.
Are these changes tested?
Yes:
in_list.sltcoveringIN/NOT INwith signed zero for DOUBLE and FLOAT columns. It fails on main and passes with this change.simplify_inlist_signed_zero: short float lists containing zero are now inlined, so the test evaluates them instead of expecting them unchanged.cargo test -p datafusion-optimizerpasses, clippy and fmt are clean.Are there any user-facing changes?
No API changes. Short float IN lists containing zero now match
-0.0and0.0the same way=does.