kszucs commented on code in PR #45001:
URL: https://github.com/apache/arrow/pull/45001#discussion_r3644315347
##########
cpp/src/arrow/compute/key_hash_internal.cc:
##########
@@ -299,10 +299,11 @@ void Hashing32::HashBit(bool combine_hashes, int64_t
bit_offset, uint32_t num_ke
template <bool T_COMBINE_HASHES, typename T>
void Hashing32::HashIntImp(uint32_t num_keys, const T* keys, uint32_t* hashes)
{
- constexpr uint64_t multiplier = 11400714785074694791ULL;
+ constexpr uint64_t kMultiplier = 11400714785074694791ULL;
+ constexpr uint64_t kAddend = 9756277977048271785ULL;
for (uint32_t ikey = 0; ikey < num_keys; ++ikey) {
- uint64_t x = static_cast<uint64_t>(keys[ikey]);
- uint32_t hash = static_cast<uint32_t>(BYTESWAP(x * multiplier));
+ uint64_t x = static_cast<uint64_t>(keys[ikey]) + kAddend;
+ uint32_t hash = static_cast<uint32_t>(BYTESWAP(x * kMultiplier));
Review Comment:
Reverted — HashIntImp/kAddend in key_hash_internal.cc are back to their
original form, so Hashing32/Hashing64 are unchanged for hash-join/group-by.
Instead, hash32/hash64 now remap a valid row's hash away from 0 locally in
scalar_hash.cc's own output (after calling the shared, unmodified
HashMultiColumn), so the null/0-collision fix is scoped to just these two new
kernels.
--
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]
