andygrove opened a new issue, #6984: URL: https://github.com/apache/datafusion-comet/issues/6984
### What is the problem the feature request solves? `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](https://github.com/apache/datafusion-comet/blob/7d8d9b1544b734c85a93cca2cc9d5380ad70accd/native/spark-expr/src/hash_funcs/utils.rs#L334-L388), [L389-L426](https://github.com/apache/datafusion-comet/blob/7d8d9b1544b734c85a93cca2cc9d5380ad70accd/native/spark-expr/src/hash_funcs/utils.rs#L389-L426)) and once for the fixed-size variant ([L433-L485](https://github.com/apache/datafusion-comet/blob/7d8d9b1544b734c85a93cca2cc9d5380ad70accd/native/spark-expr/src/hash_funcs/utils.rs#L433-L485), [L486-L521](https://github.com/apache/datafusion-comet/blob/7d8d9b1544b734c85a93cca2cc9d5380ad70accd/native/spark-expr/src/hash_funcs/utils.rs#L486-L521)). - The Map arm ([L1110-L1219](https://github.com/apache/datafusion-comet/blob/7d8d9b1544b734c85a93cca2cc9d5380ad70accd/native/spark-expr/src/hash_funcs/utils.rs#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 7d8d9b1544b7. -- 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]
