Clarify `rehash` setting in hash utils
- Dominant language
- Rust
- Stars
- 9.3k
- Forks
- 2.4k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 344
Description
### Is your feature request related to a problem or challenge?
In our hash util functions, we have a `rehash` argument across many of them:
https://github.com/apache/datafusion/blob/b80bf2ca8ef74900fee96a1cc169bdedf53b36fc/datafusion/common/src/hash_utils.rs#L185-L190
https://github.com/apache/datafusion/blob/b80bf2ca8ef74900fee96a1cc169bdedf53b36fc/datafusion/common/src/hash_utils.rs#L230-L235
https://github.com/apache/datafusion/blob/b80bf2ca8ef74900fee96a1cc169bdedf53b36fc/datafusion/common/src/hash_utils.rs#L282-L287
It's not clearly obvious why we do this from the code alone; it seems it used to be named `multi_col` and would be true if we needed to hash multiple columns, but was changed in #6816 to also skip rehash if it is the first column, for performance reasons.
- It seems dictionary function also still calls it `multi_col`
I also found it confusing how certain hash functions don't have a rehash parameter; specifically the nested types such as list, struct, etc.
https://github.com/apache/datafusion/blob/b80bf2ca8ef74900fee96a1cc169bdedf53b36fc/datafusion/common/src/hash_utils.rs#L447-L451
https://github.com/apache/datafusion/blob/b80bf2ca8ef74900fee96a1cc169bdedf53b36fc/datafusion/common/src/hash_utils.rs#L475-L479
https://github.com/apache/datafusion/blob/b80bf2ca8ef74900fee96a1cc169bdedf53b36fc/datafusion/common/src/hash_utils.rs#L510-L515
### Describe the solution you'd like
Add some documentation explaining why we have a `rehash` parameter across the functions. Also look into adding `rehash` parameter for those hash functions missing them. If this parameter was omitted on purpose for such functions, leave an explanation of why this is the case.
### Describe alternatives you've considered
_No response_
### Additional context
_No response_
Contributor guide
Research direction
Start in datafusion/common/src/hash_utils.rs at the hash functions around lines 185-190, 230-235, and 282-287, then review the dictionary and nested-type functions around lines 447-451, 475-479, and 510-515. Check the history of #6816 to understand the rehash behavior. Done means the rehash parameter is documented consistently, and missing parameters are added or their intentional omission is explained.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100