Minor: Move hash utils to common #7684
Conversation
There was a problem hiding this comment.
Thank you @jayzhan211
I double checked and this change adds no new dependencies:
https://crates.io/crates/half/reverse_dependencies is already a dependency on arrow as is ahash: https://crates.io/crates/ahash/reverse_dependencies?page=3
| @@ -28,7 +28,6 @@ pub mod equivalence; | |||
| pub mod execution_props; | |||
| pub mod expressions; | |||
| pub mod functions; | |||
| pub mod hash_utils; | |||
There was a problem hiding this comment.
I think this is technically a breaking change if someone used hash_utils directly in their code.
Perhaps we could leave a pointer like
| pub mod hash_utils; | |
| // backwards compatibility | |
| pub use datafusion_common::hash_util; |
But I don't think that is critical
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
jayzhan211
force-pushed
the
mvhsh2common
branch
from
954dfd7
to
8d5aa69
Compare
|
how to re-run CI without code changes? |
You can go to the job and click rerun: https://github.com/apache/arrow-datafusion/actions/runs/6345797930/job/17238371168?pr=7684
|
|
|
It must be related to permissions. Another way to do so is to merge up from main ( |
* move hash utils to common Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * support backward compatibility Signed-off-by: jayzhan211 <jayzhan211@gmail.com> --------- Signed-off-by: jayzhan211 <jayzhan211@gmail.com> Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Which issue does this PR close?
Closes #.
Rationale for this change
While working on #7629, I found
create_hashesis placed to physical-expr.Hash utils only import functions from arrow, std, and df-common. It seems it should be placed to df-common not physical-expr.
What changes are included in this PR?
We can have hash_utils from df-common.
Are these changes tested?
Nope, only import is modified, no need to add additional tests.
Are there any user-facing changes?