You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Reuse hash_list_primitive! for string/binary list elements and factor out map-entry hashing loops #6984
native/spark-expr/src/hash_funcs/utils.rs hand-expands the same null-aware hashing loop in several places:
Inside hash_list_with_primitive_elements!, the Utf8 and Binary arms repeat the null/no-null double loop that hash_list_primitive! already provides, once for the offsets variant (L334-L388, L389-L426) and once for the fixed-size variant (L433-L485, L486-L521).
The Map arm (L1110-L1219) repeats a null-aware loop four times, for (Utf8, Int32), (Int32, Utf8), (Utf8, Utf8) and (Int32, Int32), differing only in the downcast type and .to_le_bytes() on the integer side.
The copies are consistent today, but any change to null handling in hashing has to be made in eight or more places, and hash results must match Spark exactly.
Describe the potential solution
Call hash_list_primitive! for Utf8 and Binary elements (with an identity transform) in both the offsets and fixed-size variants, and delete the hand-written loops.
For the Map arm, add a hash_map_entries! helper macro, or a generic function taking key/value accessor closures that produce impl AsRef<[u8]>, and instantiate it for the four type pairs.
Verify with the existing murmur3/xxhash64 tests and a benchmark that inlining is preserved.
Additional context
This is self-contained and a good first issue. Line references are pinned to commit 7d8d9b1.
Activity
med9110 commentedon Oct 16, 2026
Hi @andygrove, I'd be interested in contributing to #6984.
I propose starting with the string and binary list hashing paths, reusing
hash_list_primitive!to remove the duplicated null-aware loops.I'll validate that Murmur3 and xxHash64 results remain unchanged and leave the map-entry refactoring for a separate PR.
The issue still has
requires-triage. Would this focused first step be appropriate?