apache / apache/datafusion

Clarify `rehash` setting in hash utils

Open
#20,150 0 comments 0 reactions 0 assignees View on GitHub
enhancement
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.