feat: route compatible xxhash64 args through SparkXxhash64

[sam-1112]

Which issue does this PR close?

Part of #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:

cd native && cargo test -p datafusion-comet-spark-expr xxhash64
./mvnw test -Dtest=none -Dsuites="org.apache.comet.CometHashExpressionSuite"

[andygrove]

Thanks @sam-1112. Could you share benchmark results?

[sam-1112]

Thanks @andygrove . I expanded the existing Criterion benchmark to cover the compatible type families and the fallback paths involved in this change.

Benchmark coverage

The benchmark contains 17 cases covering batch size, null distribution, compatible type families, and fallback paths.

The multi-column cases hash an Int64 + Utf8 + Float64 key using seed 42 across:

• 8,192, 65,536, and 524,288 rows
• No nulls, 10% sparse nulls, and all-null inputs

Additional 8,192-row cases cover:

• Primitive Int64
• Utf8 + Binary
• Decimal128(10, 2) + Decimal128(38, 10)
• Dictionary<Int32, Utf8>
• List<Int32> with 10 elements per row
• Map<Utf8, Int32> with 10 entries per row
• Struct<Int32, Utf8>
• Int64 with custom seed 7

The seed-42 compatible cases exercise the new SparkXxhash64 route. The struct and custom-seed cases exercise the retained Comet fallback path.

Multi-column batch-size and null-distribution results

Lower execution time is better. Negative changes indicate improvement.

Rows	Null distribution	main time	PR time	Change (95% CI)	Result
8,192	No nulls	34.059–35.348 µs	30.711–30.830 µs	−11.33% to −8.62% (estimate: −9.92%)	Improved
8,192	Sparse nulls (10%)	36.277–36.843 µs	34.888–35.794 µs	−14.69% to −5.37% (estimate: −9.32%)	Improved
8,192	All null	13.730–13.768 µs	11.554–11.753 µs	−16.12% to −15.16% (estimate: −15.76%)	Improved
65,536	No nulls	247.41–253.53 µs	230.53–234.15 µs	−7.88% to −6.00% (estimate: −6.99%)	Improved
65,536	Sparse nulls (10%)	279.18–283.42 µs	273.89–277.78 µs	−2.99% to +0.16% (estimate: −1.43%)	No significant change
65,536	All null	110.86–111.10 µs	90.088–90.685 µs	−18.98% to −18.61% (estimate: −18.80%)	Improved
524,288	No nulls	2.0139–2.0662 ms	1.8504–1.8736 ms	−10.12% to −7.61% (estimate: −8.71%)	Improved
524,288	Sparse nulls (10%)	2.2451–2.2981 ms	2.2126–2.2875 ms	−2.90% to +1.32% (estimate: −0.89%)	No significant change
524,288	All null	898.35–911.73 µs	731.02–735.89 µs	−21.99% to −16.92% (estimate: −19.27%)	Improved

Type-family and fallback-path results

This table shows the latest paired run using a freshly captured baseline
and the same filtered case order on both revisions.

Route	Shape	main time	PR time	Change (95% CI)	Result
Compatible	Primitive Int64	10.780–10.880 µs	8.492–8.645 µs	−23.13% to −21.67% (estimate: −22.35%)	Improved
Compatible	Utf8 + Binary	35.306–35.361 µs	37.734–38.508 µs	+6.69% to +8.42% (estimate: +7.45%)	Regressed in this run
Compatible	Narrow + wide Decimal128	23.178–23.629 µs	21.054–22.852 µs	−12.23% to −7.70% (estimate: −10.14%)	Improved
Compatible	Dictionary<Int32, Utf8>	12.921–13.131 µs	8.483–8.909 µs	−36.30% to −33.06% (estimate: −34.65%)	Improved
Compatible	List<Int32> × 10	154.07–154.77 µs	144.16–145.99 µs	−5.78% to −3.78% (estimate: −4.80%)	Improved
Compatible	Map<Utf8, Int32> × 10	626.28–641.70 µs	617.40–633.18 µs	−9.39% to −3.55% (estimate: −6.15%)	Improved
Fallback	Struct<Int32, Utf8>	26.921–27.027 µs	27.131–27.759 µs	−3.99% to +2.26% (estimate: −0.35%)	No significant change
Fallback	Int64, custom seed 7	9.608–9.642 µs	9.660–9.733 µs	+0.17% to +0.84% (estimate: +0.52%)	Within Criterion's noise threshold

Repeatability

I repeated the type-family comparison because the local macOS runs showed cross-process variation. The following changes were reproducible:

• Primitive Int64: approximately 15–22% faster
• Narrow + wide Decimal128: approximately 9–10% faster
• Dictionary: approximately 34–35% faster
• All-null multi-column cases: approximately 14–19% faster

Some cases changed direction between independent runs:

• Utf8 + Binary: approximately −2% to +7.5%
• List<Int32>: unchanged to approximately 4.8% faster
• Map<Utf8, Int32>: approximately 6.1% faster to 4.1% slower
• Struct fallback: approximately 6.4% faster to 2.7% slower
• Custom-seed fallback: unchanged in two comparisons, with one noisy run reporting approximately 8.7% slower

Because these are separate Criterion processes on a local macOS machine, I would treat the inconsistent cases as inconclusive rather than claim a reproducible improvement or regression. A dedicated Linux benchmark host would be appropriate if we need a firmer conclusion for those cases.

Overall, the clearest reproducible wins are primitive, Decimal128, and dictionary inputs. The fallback paths do not show a consistent material change.

[sunchao]

Correctness

Reviewed the complete six-file change at d2ebecd7 against fad62309. Previously, every xxhash64 call used Comet's kernel. This change delegates compatible inputs with Spark's default seed 42 to datafusion-spark, while retaining the original kernel for other seeds and unsupported shapes. No new or remaining verified P1/P2 findings.

Compared the implementation with the maintained Spark 3.5 and 4.0 hash expressions and XXH64. The adapter correctly removes Catalyst's trailing seed from upstream arguments, preserves signed integer widening and byte order, normalizes signed zero, skips nulls without advancing the seed, and chains columns in order. Empty byte strings remain distinct from empty collections. Structs, nested dictionaries and Time64 retain the fallback that preserves Comet's existing behavior.

The new native comparisons also cover wide decimals, but those do not establish Spark support: Spark's wide-decimal encoding differs, and the unchanged Scala serializer still rejects precision above 18 recursively. Unusual NaN payloads likewise retain an existing native/Spark difference. This PR does not introduce it.

Validation

The 49 added native test definitions cover both delegated and fallback paths. They were not executed locally. Scala additions use column-backed inputs, so constant folding does not remove the hash evaluation. Local source checks passed. An isolated comparison of the exact hashing algorithms produced 567 matching cases and reproduced 119 intentionally selected pre-existing NaN/wide-decimal differences across seven seeds. That probe did not execute the Arrow/DataFusion UDF, Spark or JNI.

At 2026-09-15 18:52 UTC, CI and CodeQL require workflow approval and have no jobs. Only labeling passed. Integration validation remains pending. Maintained Spark 3.4/4.1 source branches were unavailable, so this review makes no source-compatibility claim for those versions.

Performance

The expanded benchmark covers 17 cases across row counts, null densities, type families and fallback paths, with input construction outside the measured iteration. The author's measurements report repeatable gains for primitive, dictionary and decimal cases. The latest string/binary pair is 7.45% slower, with a reported 95% interval of +6.69% to +8.42%, but independent runs range from about 2% faster to 7.5% slower. Map and fallback measurements also vary. I treat these cases as inconclusive, not as demonstrated speedups or a stable regression. The report does not pin the executable/build or provide raw run artifacts, and I have not independently reproduced the timings.

The adapter adds per-batch type inspection, argument/field allocations and reference-count operations. Configuration initializes once. I found no newly introduced per-row adapter allocation. Existing optimized struct handling stays on Comet's fallback, and compatible nested-list paths already used recursive hashing.

Design

The guarded delegation is appropriate for incremental consolidation. Unconditional reuse would change nullable-struct and nested-dictionary behavior, while removing the original kernel would also affect arbitrary seeds and other callers. Keeping serialization, result typing and shared hash consumers unchanged confines the change to the intended expression path.

The tests explicitly demonstrate why incompatible shapes must stay on the fallback, which makes the boundary easier to maintain when upstream behavior improves.

Abstraction & complexity

The invocation helper, recursive compatibility predicate and cached configuration each have a concrete purpose. The predicate keeps the safety rules in one place. The test module stays outside production builds and covers both equivalence and known divergence. I found no unnecessary abstraction or actionable design simplification.

[sunchao]

Re-reviewed 8c1e2159 against 347d8cf3. This update merges main without changing the six-file xxhash64 contribution, its dependency versions or the serializer. The seed-42 routing and fallbacks remain intact. No new or remaining P1/P2 findings. My existing approval stands.

Current native CI passed all 49 added xxhash64 differential tests, within 1,500 passing tests and five skipped. The executed merge eda3070a has parents [347d8cf3, 8c1e2159] and a tree identical to this head. This supplies actual Arrow/DataFusion test execution. The previous isolated algorithm probe remains separate evidence. Spark 4.1 integration checks are still running, and no local Spark/JNI run was performed.

The benchmark implementation and report are unchanged. Their missing build provenance and variable string/binary, map and fallback results remain qualified as before. Neither new timings nor a new speedup claim were added. The existing NaN-payload difference and Spark fallback for decimal precision above 18 are unchanged.

At September 15, 20:34 UTC, CI shows 19 successful, 13 skipped and three running checks, with no failures. The merged CI policy runs Comet suites only on Spark 4.1 in the PR tier. Other runtime profiles wait for the merge queue or explicit labels.

[sunchao]

Rechecked 32bc2032 against 58ab5f61 and the previously reviewed 8c1e2159. The full authored diff and all six PR files are unchanged. The 140 incremental paths come from the base update. Seed/type guards, Spark serde, and shared HLL/shuffle hash paths retain the reviewed behavior. No new or remaining verified P1/P2 finding. The existing approval stands.

The 17-case native benchmark is unchanged, and the inherited fixture-helper change leaves its 0%, 10%, and 100% null inputs identical. The author's variable timing results remain historical. I did not rerun benchmarks or infer a new performance gain.

Validation was source comparison against maintained Spark 3.5/4.0, retained locked upstream source, and deterministic fixture checks. The previous native CI passed all 49 differential tests on the prior merge, not this revision. Current-head CI and CodeQL remain action_required with zero jobs. Only labeling passed. No local Spark/JNI suite was run, and maintained Spark 3.4/4.1 sources remain unavailable.