andygrove opened a new pull request, #6413:
URL: https://github.com/apache/datafusion-comet/pull/6413
## Which issue does this PR close?
Part of #6385: the "canonicalize NaN in `hash` / `xxhash64`" step.
This is stacked on #6400, so until that merges this diff also contains
#6400's commits. Only the last commit, 71d6e7be9, is new here. I'll rebase once
#6400 is in.
## Rationale for this change
Spark hashes a float through `doubleToLongBits` or `floatToIntBits`. Both
canonicalize NaN, so every NaN hashes alike. Comet hashed the raw bits. A NaN
with the sign bit set therefore hashed differently from Spark. Negating a NaN
produces one on every platform, and on x86-64 every NaN produced by arithmetic
has the sign bit set:
| Query | Spark | Comet before |
| --- | --- | --- |
| `SELECT hash(-d), xxhash64(-d) FROM t WHERE id = 3` | `-1281358385,
-3127944061524951246` | `-1489914710, 9200374361256412029` |
The hash serde reports float arguments as compatible, so these queries did
not fall back to Spark.
## What changes are included in this PR?
- `float_semantics::hash_input` now returns `normalize_float`: `-0.0` hashes
as `0.0`, as before, and every NaN hashes as the canonical NaN. Since #6400
every float hash path goes through `hash_input`. That covers `hash`,
`xxhash64`, list, struct and dictionary elements, the native shuffle's hash
partitioner, and `approx_count_distinct`.
- At the default seed, `xxhash64` handed "compatible" arguments to
datafusion-spark's `SparkXxhash64`, which also hashes a NaN's raw bits. Floats,
and types containing them, now stay on Comet's kernel. `xxhash64_diff.rs` lists
the new divergence with a test, like its other known differences from upstream.
- `approx_count_distinct` no longer runs a separate float normalization pass
before hashing, because `xxhash64` now gives the same hash on its own. Spark's
HLL normalizes and then hashes, and gets the same result.
Effects beyond the two functions:
- Native shuffle hash partitioning now sends a row with a non-canonical NaN
key to the partition Spark picks. This matters when a Comet exchange has to
agree with a Spark exchange. Joins were not affected, because Spark normalizes
join keys before the exchange.
- Runtime bloom filters hash their keys with `xxhash64`. A filter built by
one engine and probed by the other now agrees on NaN keys.
The extra NaN check costs about 0.17 ns per value in the float hash loop. On
an M3 Max a batch of 8192 doubles goes from 4.7 µs to 6.1 µs, still about 2x
faster than before #6400.
datafusion-spark's `SparkXxhash64` has the same gap, which I can report
upstream.
## How are these changes tested?
- `hash.sql` gains NaN rows, compared against Spark. One query hashes `d`,
`-d`, `f` and `-f` with `hash` and `xxhash64`. Another hashes arrays and
structs of negated values. Against the old library the first query fails with
the values in the table above. It passes now.
- A new `CometNativeShuffleSuite` test compares `spark_partition_id()` per
row against Spark for float keys, including negated NaNs, and asserts that the
exchange ran natively. With the old hash, the NaN row went to partition 0,
where Spark puts it in partition 5.
- New unit tests, each of which fails under the old rule:
- Spark's hash values for canonical, sign-bit and payload NaNs in the
`murmur3` and `xxhash64` float tests.
- Both hashes of non-canonical NaNs inside lists, large lists, fixed-size
lists, structs and dictionaries.
- `approx_count_distinct` counting the two zeros as one value and all NaNs
as one.
- The `xxhash64` expression path keeping floats on Comet's kernel.
- Results on macOS aarch64 with the default Spark 4.1 profile:
- Unit tests: 1025 in spark-expr, 552 in core, 171 in shuffle.
- `CometSqlFileTestSuite`: 578.
- `CometHashExpressionSuite` and `CometNativeShuffleSuite`: 100.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
