New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

GH-17211: [C++] Add `hash32` and `hash64` scalar compute functions #45001

Open

kszucs wants to merge 38 commits into apache:main from kszucs:scalar-hash

Member

kszucs commented Dec 11, 2024 •

edited

Loading

Rationale for this change

Support for calculating elementwise hashes.

The PR adds to scalar functions hash32() and hash64() using the existing internal hashing machinery.

What changes are included in this PR?

Continuation of #39836 with the following changes:

Use column oriented hash-combine rather than flattening nested elements
Support arbitrary nesting levels with an optimization that only hash child arrays if they are also nested

Are these changes tested?

Partially, working on a proper testing suite.

Are there any user-facing changes?

There is a new compute kernel hash_64 available.

GitHub Issue: [C++][Compute] Add scalar_hash function #17211

github-actions bot added Component: C++ Component: Documentation awaiting committer review labels

kszucs commented

View reviewed changes

cpp/src/arrow/compute/light_array_internal.h Outdated Show resolved Hide resolved

kszucs commented

View reviewed changes

cpp/src/arrow/compute/light_array_internal.h Outdated Show resolved Hide resolved

github-actions bot added awaiting changes and removed awaiting committer review labels

kszucs commented

View reviewed changes

cpp/src/arrow/compute/kernels/scalar_hash.cc Outdated Show resolved Hide resolved

Member Author

kszucs commented Dec 11, 2024

Seems like we generate the same hash for both NULL and 0 which is not ideal.

In [1]: import pyarrow as pa

In [2]: import pyarrow.compute as pc

In [3]: pc.hash_64([None])
Out[3]:
<pyarrow.lib.UInt64Array object at 0x124247be0>
[
  0
]

In [4]: pc.hash_64([0])
Out[4]:
<pyarrow.lib.UInt64Array object at 0x1033027a0>
[
  0
]

kszucs commented

View reviewed changes

cpp/src/arrow/compute/kernels/scalar_hash.cc Outdated Show resolved Hide resolved

github-actions bot added Component: Python awaiting change review and removed awaiting changes labels

kszucs commented

View reviewed changes

python/pyarrow/tests/test_compute.py Outdated Show resolved Hide resolved

kszucs marked this pull request as ready for review

December 11, 2024 17:41

github-actions bot added awaiting changes and removed awaiting change review labels

kszucs changed the title ~~GH-17211: [C++] Add hash_64 scalar compute function~~ GH-17211: [C++] Add hash_64 scalar compute function

github-actions bot added awaiting change review awaiting changes and removed awaiting changes awaiting change review labels

zanmato1984 requested changes

View reviewed changes

Contributor

zanmato1984 left a comment

Some first glance comments. I'll look into more details later.

cpp/src/arrow/compute/api_scalar.h Outdated Show resolved Hide resolved

cpp/src/arrow/compute/kernels/CMakeLists.txt Outdated Show resolved Hide resolved

cpp/src/arrow/compute/kernels/CMakeLists.txt Outdated Show resolved Hide resolved

cpp/src/arrow/compute/kernels/scalar_hash.cc Outdated Show resolved Hide resolved

docs/source/cpp/compute.rst Outdated Show resolved Hide resolved

cpp/src/arrow/compute/api_scalar.cc Outdated Show resolved Hide resolved

github-actions bot added awaiting changes and removed awaiting change review labels

kszucs changed the title ~~GH-17211: [C++] Add hash_64 scalar compute function~~ GH-17211: [C++] Add hash32 and hash64 scalar compute functions

github-actions bot added awaiting change review and removed awaiting changes labels

kszucs added 27 commits

December 20, 2024 10:09


          minor cleanup

6be6c98


          wrong include

55fd33b


          add hash32 function

a0602f8


          add hash32 function and address review comments

d69754c


          update testing suite

db4bc09


          generated struct tests

e5791a6


          prepare adding hash32 tests

77003b0


          add tests for hash32, dictionary encoded type, extension type

185b034


          ensure that NULL and 0 hashes to different numbers

77970dd


          fix the benchmarks

1ea0781


          update docs to highlight 0 return for NULL values

63af150


          use resize instead of reserve

fc09437


          fix R compute function filtering test

b1544cc


          address review comments

1ddf157


          update boolean test case

6c27931


          update exec checks

c831ee3


          use Hasher::kHashBatchTempStackUsage

d0e9942


          ensure that slices properly work

b27d1b1


          add tests for fixed_size_binary()

061ff68


          add rudimentary quality tests


          test temporal types too

c7703e6


          test temporal and decimal types

c42ea3d


          add type matcher to explicitly raise NotImplemented for unsupported t…

daae780

…ypes


          update error message

c65d5c9


          update docs

7ceca82


          set a tolerance for the randomly generated numbers

4590c30


          make msvc happy

e6669fc

kszucs force-pushed the scalar-hash branch from f87a6de to e6669fc Compare

December 20, 2024 09:11

Member Author

kszucs commented Dec 20, 2024

Just asking - any idea if there is line of sight to this issue being closed? I am interested because, among other things, I believe this Arrow functionality is the underlying reason why pandas cannot currently group by a pyarrow-backed struct series.

The PR should be ready now. Do you have a reference to the relevant pandas issue perhaps?

Contributor

a-reich commented Dec 21, 2024

@kszucs I couldn’t find an existing pandas issue so I created one, see pandas-dev/pandas#60594 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting change review Component: C++ Component: Documentation Component: Python Component: R