fix: [branch-1.0] keep the dictionary hash fast path off nested and reseeded buffers (#5757)

[andygrove]

Backport of #5757 to branch-1.0.

Cherry-picked from 92ad99e97482c861062f52372b172320197c2001 with no conflicts and no
modifications — the diff is byte-identical to the original PR.

Which issue does this PR close?

Closes #5756 on branch-1.0.

Rationale for this change

The dictionary fast path hashes each distinct dictionary value once and reuses that result for
every key. It was selected by the column's position, i == 0, and it restarted from a hardcoded
seed of 42. Both parts are wrong:

• create_hashes_internal! also runs on recursion, so a dictionary nested in a list, struct or
  map arrives as the only column of its call and looks like a first column even though the buffer
  already holds the hash accumulated for earlier elements of that row. That hash was discarded, so
  a dictionary-encoded list element hashed differently from the identical decoded value.
• The hardcoded 42 is wrong whenever the caller supplies its own seed, which hash(col, seed) and
  xxhash64(col, seed) allow, so even a genuine first column disagreed with its decoded form for a
  non-default seed.

For a shuffle partitioning key that means equal keys can reach different partitions, breaking
grouping and joins; the hash() and xxhash64() SQL functions are affected too.

What changes are included in this PR?

• The reuse is valid exactly when every row carries the same incoming hash, so that is what is
  checked now. The check is ANDed onto the existing i == 0 test rather than replacing it, which
  stays conservative: a nested dictionary that reaches the macro as the only column of its
  recursive call is rejected whenever the buffer already holds differing accumulated hashes.
• The per-value hashes start from the seed the buffer actually holds instead of an assumed 42.
• Both changes are in murmur3 and xxhash64, which share this structure.

A top-level dictionary column keeps the optimisation. The uniformity check is a scan of the hash
buffer and is measurable, so it is done inside the dictionary arm — only dictionary columns pay
for it.

How are these changes tested?

Same tests as the original PR, verified locally on branch-1.0:

• cargo test -p datafusion-comet-spark-expr passes 615 + 3 tests.
• cargo check --workspace --all-targets is clean across all six crates, and
  cargo clippy -p datafusion-comet-spark-expr --all-targets -- -D warnings and
  cargo fmt --all -- --check pass.
• The five new regression tests were confirmed to fail on branch-1.0 with the fix reverted and
  the tests kept, reproducing the exact values from the original PR: the dictionary list element
  hashes 3853467749 instead of 1401423033, and the top-level dictionary test fails on the
  seed-7 case. So the defect is present on branch-1.0 and this backport is what removes it.

The patch is byte-identical to 92ad99e9, but the dependency environment is not the same as the
one it was merged into: branch-1.0 resolves DataFusion 54.1.0 and Arrow 58.4.0, whereas main
at that commit resolves DataFusion 55.0.0 and Arrow 59.3.0. The cherry-pick still applied and
compiled unmodified because the Arrow surface these three files touch is unchanged between those
versions, so no API adaptation was needed — but that is an observation about these files, not a
property of the two branches.

No microbenchmark has been re-run against branch-1.0's Arrow 58.4 dependency set; the
performance characterisation above is inherited from #5757, which measured on Arrow 59.3.

Are there any user-facing changes?

Dictionary-encoded values now hash identically to their decoded form inside nested types and for
non-default seeds. Hash values for those cases change, which is the point of the fix — a
top-level dictionary column with the default seed is unaffected.

[viirya]

Thanks @andygrove for cherry-picking this!

[sunchao]

Correctness

Reviewed head 42182b273651745687ac98923cb0e4a84230624a against branch-1.0 base 008cd88ee2a8aa6cdc6cbff81686affc22497598. I found no P1/P2 issue in the full three-file backport.

Previously, dictionary reuse depended on column position and restarted dictionary values at 42. A recursive list element appears as column zero even after an earlier element has changed its row hash. A custom Catalyst seed also need not be 42. The change checks that the incoming buffer is uniform and initializes dictionary hashes from that actual value. Nonuniform buffers follow the existing decoded path, retaining each row's seed. The additional first-column restriction remains conservative and safe.

This matches the relevant maintained Spark 3.5 (5947fd6e) and 4.0 (03f28fc4) semantics: null preserves the incoming hash, and arrays, structs and supported map paths chain values in order. Null dictionary keys skip the update. Keys referencing null values produce the unchanged incoming seed. Single-row recursive buffers can safely retain reuse because the helper now starts from their accumulated hash. I traced the expression seed serialization and shuffle callers. The custom seed is an internal expression parameter, while an ordinary SQL argument to hash is another value to hash. Type support, fallback gates, hash arithmetic and overflow behavior are unchanged. Maintained Spark 3.4 and 4.1 sources were unavailable, so no source compatibility claim is made for those versions.

The patch is byte-identical to the cited upstream commit, but its dependency environment differs: this backport resolves DataFusion 54.1.0 and Arrow 58.4.0. Upstream 92ad99e9 resolves DataFusion 55.0.0 and Arrow 59.3.0. Please correct the description's same-dependency statement to reflect those pins. Current native CI checked out merge f17386ef328dc347a6df90d988a807fbb05332f1, whose parents are the exact base/head above and whose complete tree equals the reviewed head. It passed 859 tests, with four skipped, including all five added dictionary regressions. This is independently inspected CI execution, not a local Cargo or JVM rerun. At 2026-09-09T20:46:20.512933+00:00, the snapshot had 29 successful checks, 7 skipped, 26 running and 1 queued. Full product CI remains pending.

Performance

The uniformity scan runs only in the dictionary arm and only for the first column of its current call. Primitive/string columns and later dictionary columns do not pay that scan. Uniform buffers retain one hash per dictionary value plus key lookup. Nonuniform buffers require the existing decoding allocation because one cached result cannot represent different incoming seeds. The added scan is linear and short-circuits on divergence. Repeated hashing of a dictionary's values in singleton nested calls already exists in the base. I did not run a benchmark on this backport. Could you include a focused before/after microbenchmark on its Arrow 58.4 dependency set, covering small and large dictionaries with uniform and nonuniform incoming seeds? The additional buffer scan is most visible for cheap, low-cardinality values, so that case should accompany the string case. Compare equivalent logical outputs when measuring a corrected path.

Design

The guard sits beside dictionary dispatch, while each algorithm-specific helper consumes the actual seed. This repairs recursive and custom-seed behavior without changing callers or introducing a separate recursion mode. The authoritative base is the head's direct parent, and the synthetic merge introduces no additional source changes. The intended behavioral change is that affected dictionary encodings now hash like their decoded values. A top-level dictionary with the default seed keeps its existing result.

Abstraction & complexity

The shared macro applies the same eligibility check across all eight existing dictionary key types. No public API, dependency, persistent state or new abstraction is added. The five regressions distinguish nested seed propagation, custom uniform seeds and nonuniform buffers containing both kinds of null. The change is small enough to keep the two algorithm helpers direct. A wider refactor is unnecessary for this correction.

[andygrove]

Thanks @sunchao. You're right about the pins, and I've corrected the description. branch-1.0 resolves DataFusion 54.1.0 and Arrow 58.4.0 while main at 92ad99e9 is on 55.0.0 and Arrow 59.3.0, so the "same DataFusion version" line was just wrong. The cherry-pick still applied unmodified because the Arrow surface these three files touch didn't change between those versions, which is what the description now says instead.

Your review also caught a second inaccuracy I'd written: I claimed the uniformity check replaced the column-index test, but it's ANDed onto it in utils.rs, so the fast path is strictly narrower than before rather than differently scoped. That's reworded too.

On the microbenchmark, I haven't run one against the 58.4 dependency set, and the description now says so rather than implying the numbers carry over from #5757. Would you rather I produce that before this merges, or is it reasonable to land the correctness fix here given the patch is byte-identical to what's already on main and take the benchmark separately?