sam-1112 opened a new pull request, #5960: URL: https://github.com/apache/datafusion-comet/pull/5960
## Which issue does this PR close? Part of https://github.com/apache/datafusion-comet/issues/5103 (xxhash64-consolidation only). ## Rationale for this change #5103 asked Comet to stop maintaining a parallel native `xxhash64` implementation where `datafusion-spark::SparkXxhash64` already matches Spark. A full swap is not safe: `SparkXxhash64` hardcodes seed `42`, does not push a parent null mask into struct children (#5753), restarts nested-dictionary hashes from 42 when a list/map element is hashed as a one-row first column, and does not dispatch `Time64`. This PR therefore uses hybrid routing: compatible arguments at Spark's default seed go through `SparkXxhash64`; everything else stays on Comet's kernel. `murmur3` is unchanged. `create_xxhash64_hashes` is kept for `approx_count_distinct` (HLL++) and for the fallback path. ## What changes are included in this PR? - `spark_xxhash64` now delegates to `SparkXxhash64` when the trailing Catalyst seed is `42` and every argument type is compatible. - Compatibility is type-driven: primitives, strings/binary, dates/timestamps, Decimal128 (both widths), top-level dictionaries, lists (including `FixedSizeList` / `LargeList`), and maps. - Comet's kernel is retained for: - a non-default seed - `Struct` (and any type containing one), so hidden child values of a NULL struct do not affect the hash - a `Dictionary` nested in a list/map - `Time64` - Shared hash macros used by `murmur3` and `create_xxhash64_hashes` are left in place. - Audit notes for `xxhash64` record the 2026-09-15 upstream routing decision. ## How are these changes tested? - New differential tests in `native/spark-expr/src/hash_funcs/xxhash64_diff.rs` compare Comet's kernel against `SparkXxhash64` for primitives, both Decimal128 widths, dictionaries, lists, maps (Utf8/Int32, Int32/Utf8, Utf8/Utf8, Int32/Int32, Decimal), nested combinations, multi-column chaining, custom seeds, and the known incompatibilities (null structs, nested dictionaries, Time64). - `CometHashExpressionSuite` now selects both `hash()` and `xxhash64()` on the existing type coverage, including custom seeds, structs, arrays, maps, dictionaries, and fuzz. Local checks: ```sh cd native && cargo test -p datafusion-comet-spark-expr xxhash64 ./mvnw test -Dtest=none -Dsuites="org.apache.comet.CometHashExpressionSuite" -- 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]
