Skip to content

fix: fold -0.0 into +0.0 in IN-list filters and the distinct accumulator - #4

Open
fornwall wants to merge 1 commit into
mainfrom
float-zero-in-list-and-distinct
Open

fix: fold -0.0 into +0.0 in IN-list filters and the distinct accumulator#4
fornwall wants to merge 1 commit into
mainfrom
float-zero-in-list-and-distinct

Conversation

@fornwall

@fornwall fornwall commented Jul 20, 2026

Copy link
Copy Markdown
Owner

Which issue does this PR close?

No tracking issue — this is a follow-up to apache#22835, which introduced -0.0 → +0.0 normalization for float equality and grouping. Happy to file an issue first if preferred.

Rationale for this change

apache#22835 normalized float zeros at the sites where DataFusion hands float data to Arrow's totalOrder-based kernels (=, GROUP BY, joins, array set ops), so -0.0 and +0.0 compare and group as equal, matching IEEE 754 equality and PostgreSQL. A few paths were missed and still compare raw bits, so the engine currently disagrees with itself:

-- IN disagrees with = :
select 0.0 = -0.0;      -- true
select 0.0 IN (-0.0);   -- false (static filter compares bit patterns)

-- ...which also means the answer depends on plan shape, since the
-- simplifier rewrites single-element IN lists into = :
select 0.0 IN (-0.0, 1.0);  -- false (kept as InList)
-- vs. the equivalent = chain, which returns true

-- COUNT(DISTINCT) disagrees with the GROUP BY path of the same aggregation:
select count(distinct x) from (values (0.0), (-0.0)) t(x);           -- 1 (distinct-to-group-by rewrite)
select count(distinct x), count(x) from (values (0.0), (-0.0)) t(x); -- 2, 2 (distinct accumulator)

The second inconsistency is the same class as apache#16254 (COUNT(DISTINCT) giving different answers for the same data depending on which execution path runs).

What changes are included in this PR?

Apply the existing normalize_float_zero / normalize_float_zero_scalar helpers from apache#22835 at the remaining sites:

  • IN-list static filters: OrderedFloat32 / OrderedFloat64 fold -0.0 on construction (covering both set build and probe), and the Float16 BitmapFilter folds it in index().
  • IN-list non-constant list path: normalize the needle array and each list value before the eq kernel / comparator.
  • GenericDistinctBuffer::update_batch: normalize input arrays before inserting into the bit-based Hashable set (merge_batch delegates to update_batch).

No new helpers, no semantic policy change — only making the remaining paths agree with the semantics apache#22835 already established.

Are these changes tested?

  • New sqllogictest cases in predicates.slt: IN over ±0.0 on both the constant-list (static filter) and non-constant-list paths, and COUNT(DISTINCT) with mixed aggregates so the distinct accumulator runs.
  • The Float16 bitmap filter unit test now probes +0.0 against a -0.0 haystack entry.
  • Full sqllogictest suite and unit tests for the touched crates pass.

Are there any user-facing changes?

The queries above now return consistent results (0.0 IN (-0.0) is true; mixed-aggregate COUNT(DISTINCT) counts ±0.0 as one value). No API changes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WS4N9hte6f1W5r6EuDzobY

`=` already normalizes float zeros before comparing (IEEE 754 and
PostgreSQL both treat `-0.0` and `+0.0` as equal), and so do GROUP BY and
joins, but two paths still compared raw bits: the IN-list static filters
(hash set / bitmap) and dynamic list path, and the distinct-values
accumulator used by e.g. COUNT(DISTINCT) alongside other aggregates. This
made `0.0 IN (-0.0)` false while `0.0 = -0.0` is true, and made
`SELECT COUNT(DISTINCT x), COUNT(x) FROM (VALUES (0.0), (-0.0)) t(x)`
return 2 where the GROUP BY rewrite of the same query returns 1.

Normalize with the existing `normalize_float_zero` helpers at those sites.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WS4N9hte6f1W5r6EuDzobY
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant