Skip to content

fix(types): hash 0.0 and -0.0 to the same value - #1507

Open
MaxFreedomPollard wants to merge 1 commit into
pgdogdev:mainfrom
MaxFreedomPollard:fix/float-negative-zero-hash
Open

fix(types): hash 0.0 and -0.0 to the same value#1507
MaxFreedomPollard wants to merge 1 commit into
pgdogdev:mainfrom
MaxFreedomPollard:fix/float-negative-zero-hash

Conversation

@MaxFreedomPollard

Copy link
Copy Markdown

The Float wrapper in pgdog-config and pgdog-vector and the Double wrapper in pgdog-postgres-types compare zeros numerically, so 0.0 and -0.0 are equal, but their Hash impls hashed the raw bit pattern, which differs by the sign bit, so two equal values hash differently and a HashMap or HashSet keyed on them keeps them apart. Datum derives Hash, so this reaches the cross-shard GROUP BY buffer in backend::pool::connection::aggregate, where a -0.0 returned by one shard forms its own group instead of merging with the 0.0 from another. Each of the three Hash impls now hashes the bits of positive zero when the value is zero, the same way Postgres normalizes zero in hashfloat4 and hashfloat8. New tests cover it in all three crates (test_negative_zero_hashes_like_zero in pgdog-config and pgdog-vector, test_negative_zero_datums_hash_like_zero over Datum::Float, Datum::Double and Datum::Vector, test_double_negative_zero_groups_with_zero, and test_distinct_values_still_hash_apart so zero does not swallow other values), the two existing hash consistency tests that asserted the old behaviour now assert equality, and cargo fmt --all -- --check, cargo clippy --all-targets -- -D warnings and cargo nextest run are clean for the three crates.

The Float and Double wrappers compare zeros numerically, so 0.0 == -0.0,
but they hashed the raw bit pattern, which differs by the sign bit. That
breaks the Hash and Eq contract: a HashMap or HashSet keyed on these
values, such as the cross-shard GROUP BY buffer, puts equal keys in
separate buckets. Hash the bits of positive zero instead, matching what
Postgres does in hashfloat4 and hashfloat8.
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

2 participants