Skip to content

[branch-56] IN LIST: treat signed zeros as equal (backport #25186) - #27204

Merged
xudong963 merged 1 commit into
apache:branch-56from
alamb:alamb/backport-25186
Nov 9, 2026
Merged

xudong963 merged 1 commit into
apache:branch-56from
alamb:alamb/backport-25186

Conversation

@alamb

@alamb alamb commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

## Which issue does this PR close?

- Part of apache#26138 (shared floating-point semantics).
- Covers the IN-list regression reported in apache#27121.
- Closes apache#27121
- Closes apache#27122

## Rationale for this change

DataFusion equality treats `+0.0` and `-0.0` as equal, but the optimized
`IN` filters compare or hash floating-point bit patterns. Because the
two zeros have different encodings, an `IN` predicate could therefore
return a different result depending on whether the optimizer retained it
or rewrote it into equality comparisons.

This signed-zero limitation predates the IN-list optimization epic
apache#19241: [apache#7378](apache#7378) already
used bit-pattern equality for floating-point membership in August 2023.
[apache#22835](apache#22835) corrected
ordinary comparisons in June 2026 but did not fix `IN` membership. This
PR addresses that remaining correctness gap.

This PR makes `Float16`, `Float32`, and `Float64` `IN` predicates,
including floats inside nested and encoded values, follow the same
signed-zero equality as ordinary comparisons. The shared static-filter
factory adds the missing zero encoding before choosing a primitive
filter. Strategy selection uses the expanded physical non-null list
length, so both encodings count toward the existing cutoffs. Generic
static filters and non-static lists use the shared `normalize_floats`
and recursive `ScalarValue::normalize` APIs before comparison. Distinct
NaN payloads remain distinct, preserving the existing bit-equality
behavior for NaNs.

## What changes are included in this PR?

- Add the missing signed-zero encoding once in the shared static-filter
factory, after flattening dictionary lists.
- Select branchless, bitmap, or hash-set lookup using the expanded
physical non-null key count.
- Extend `ScalarValue::normalize` to nested and encoded values, and use
`float_semantics::normalize_floats` for arrays in dynamic comparisons
and generic static filters.
- Prevent structural literal equality from driving IN-list set-operation
rewrites for types containing floating-point values.
- Preserve null behavior, dictionary inputs, `IN`, and `NOT IN`.

## What is the testing strategy for this PR?

SQL regressions in `negative_zero.slt` cover both signed-zero
directions, static filters, dynamic scalar and array operands, nested
values, IN-list intersection/difference, `IN`, `NOT IN`, and nulls.
Focused Rust tests cover dictionary inputs, distinct NaN payloads,
encoded scalar metadata, and sliced values. Factory tests exercise both
zero signs immediately below and at each floating-point branchless
cutoff, including dispatch after adding the missing physical key. SQL
tests also check constant nested scalar self-membership; an existing
Rust test covers nested dictionary scalar inputs.

Validation after rebasing onto `upstream/main` (`4bf32cb46d`):
formatting, CI Clippy with its standard feature set, all 111 focused
Rust tests, and all 546 sqllogictest files (including Parquet
encryption) passed.

## Are there any user-facing changes?

Yes. Floating-point `IN` and `NOT IN`, including nested and encoded
values, now treat `+0.0` and `-0.0` as equal, consistently with ordinary
DataFusion equality. `ScalarValue::normalize` now applies these rules
recursively; public API signatures are unchanged.

Prepared with AI assistance.

---------

Co-authored-by: Andrew Lamb <[email protected]>
(cherry picked from commit 94486b1)
@alamb alamb mentioned this pull request Nov 7, 2026
40 of 61 tasks
@github-actions github-actions Bot added physical-expr Changes to the physical-expr crates optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt) common Related to common crate labels Nov 7, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.08487% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.27%. Comparing base (2277aa4) to head (e94791a).

Files with missing lines Patch % Lines
...atafusion/physical-expr/src/expressions/in_list.rs 79.36% 0 Missing and 13 partials ⚠️
.../physical-expr/src/expressions/in_list/strategy.rs 87.50% 0 Missing and 12 partials ⚠️
datafusion/common/src/float_semantics.rs 93.67% 3 Missing and 2 partials ⚠️
datafusion/common/src/scalar/mod.rs 80.00% 5 Missing ⚠️
Flag Coverage Δ
all-tests 83.27% <87.08%> (+<0.01%) ⬆️
sqllogictests 60.73% <77.27%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@@             Coverage Diff             @@
##           branch-56   #27204    +/-   ##
===========================================
  Coverage      83.26%   83.27%            
===========================================
  Files           1169     1169            
  Lines         474432   474679   +247     
  Branches      474432   474679   +247     
===========================================
+ Hits          395058   395283   +225     
+ Misses         55458    55457     -1     
- Partials       23916    23939    +23     
Files with missing lines Coverage Δ
...imizer/src/simplify_expressions/expr_simplifier.rs 93.60% <100.00%> (+<0.01%) ⬆️
...xpr/src/expressions/in_list/array_static_filter.rs 93.42% <100.00%> (+0.36%) ⬆️
datafusion/common/src/float_semantics.rs 97.88% <93.67%> (-2.12%) ⬇️
datafusion/common/src/scalar/mod.rs 88.26% <80.00%> (-0.02%) ⬇️
.../physical-expr/src/expressions/in_list/strategy.rs 84.43% <87.50%> (+4.14%) ⬆️
...atafusion/physical-expr/src/expressions/in_list.rs 86.55% <79.36%> (-0.09%) ⬇️

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xudong963
xudong963 merged commit 964f925 into apache:branch-56 Nov 9, 2026
40 checks passed
@xudong963

Copy link
Copy Markdown
Member

Thanks @alamb

alamb added a commit that referenced this pull request Nov 12, 2026
## Which issue does this PR close?

- Part of #24461

## Rationale for this change

Several PRs were backported to `branch-56` after the changelog was
generated in #27098, so they
are missing from `dev/changelog/56.0.0.md`.

## What changes are included in this PR?

Regenerates the changelog with `dev/release/generate-changelog.py 55.0.0
branch-56 56.0.0`. This adds entries for:

- #27243
- #27204
- #27331
- #27369
- #27368
- #27098

and updates the commit count and credits.

## What is the testing strategy for this PR?

N/A (changelog only)

## Are there any user-facing changes?

No

Co-authored-by: Claude Fable 5.2 <[email protected]>
joseph-isaacs added a commit to vortex-data/vortex that referenced this pull request Nov 13, 2026
DataFusion 56.0.0-rc1 backports apache/datafusion#27204: IN treats 0.0 and
-0.0 as equal, while other floats still compare by their bits. Vortex's set
lookup compares bits, so a pushed-down `x IN (-0.0)` missed 0.0. Add the
missing signed zero to the converted list, as DataFusion does.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01UeniBDc663cPR76jiGX4Xq
Signed-off-by: Joe Isaacs <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common Related to common crate optimizer Optimizer rules physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants