diff --git a/src/iceberg/location_provider.cc b/src/iceberg/location_provider.cc index a03f41e5f..70274edc6 100644 --- a/src/iceberg/location_provider.cc +++ b/src/iceberg/location_provider.cc @@ -89,9 +89,8 @@ std::string DirsFromHash(int32_t hash) { } std::string ComputeHash(std::string_view file_name) { - int32_t hash_value = 0; - MurmurHash3_x86_32(file_name.data(), file_name.size(), 0, &hash_value); - return DirsFromHash(hash_value); + return DirsFromHash( + static_cast(MurmurHash3_x86_32(file_name.data(), file_name.size(), 0))); } } // namespace diff --git a/src/iceberg/test/bucket_util_test.cc b/src/iceberg/test/bucket_util_test.cc index bee84a7cb..8be6989c3 100644 --- a/src/iceberg/test/bucket_util_test.cc +++ b/src/iceberg/test/bucket_util_test.cc @@ -19,23 +19,35 @@ #include "iceberg/util/bucket_util.h" +#include #include +#include +#include +#include +#include #include #include "iceberg/expression/literal.h" #include "iceberg/test/temporal_test_helper.h" #include "iceberg/util/decimal.h" +#include "iceberg/util/murmurhash3_internal.h" #include "iceberg/util/uuid.h" namespace iceberg { +static_assert(std::is_convertible_v); + // The following tests are from // https://iceberg.apache.org/spec/#appendix-b-32-bit-hash-requirements TEST(BucketUtilsTest, HashHelper) { // int and long EXPECT_EQ(BucketUtils::HashInt(34), 2017239379); EXPECT_EQ(BucketUtils::HashLong(34L), 2017239379); + const auto bucket = BucketUtils::BucketIndex(Literal::Int(34), 100); + ASSERT_TRUE(bucket.has_value()); + EXPECT_EQ(*bucket, 79); // decimal hash auto decimal = Decimal::FromString("14.20"); @@ -108,6 +120,79 @@ TEST(BucketUtilsTest, HashHelper) { EXPECT_EQ(BucketUtils::HashBytes(fixed), -188683207); } +TEST(BucketUtilsTest, BucketByteLiterals) { + struct BucketCase { + Literal literal; + int32_t bucket; + }; + const std::array cases = {{ + {.literal = Literal::String("iceberg"), .bucket = 89}, + {.literal = Literal::Binary({0, 1, 2, 3}), .bucket = 41}, + {.literal = Literal::Fixed({0, 1, 2, 3}), .bucket = 41}, + }}; + for (const auto& test : cases) { + const auto bucket = BucketUtils::BucketIndex(test.literal, 100); + ASSERT_TRUE(bucket.has_value()); + EXPECT_EQ(*bucket, test.bucket); + } +} + +TEST(BucketUtilsTest, CanonicalLittleEndianBlocks) { + constexpr size_t kMaxLength = 33; + constexpr size_t kAlignment = alignof(uint64_t); + struct HashCase { + size_t length; + uint32_t hash; + }; + constexpr std::array cases = {{ + {.length = 0, .hash = 0x00000000u}, + {.length = 1, .hash = 0x514e28b7u}, + {.length = 2, .hash = 0x70e1a2c0u}, + {.length = 3, .hash = 0x51d4d0d7u}, + {.length = 4, .hash = 0xf4c0ec39u}, + {.length = 5, .hash = 0xcca4dccbu}, + {.length = 6, .hash = 0x901cc302u}, + {.length = 7, .hash = 0x8d7e4914u}, + {.length = 8, .hash = 0xd161d673u}, + {.length = 15, .hash = 0x5bd6952du}, + {.length = 16, .hash = 0x191573ddu}, + {.length = 17, .hash = 0xcbe58dc6u}, + {.length = 32, .hash = 0xcac37638u}, + {.length = kMaxLength, .hash = 0x5460867au}, + }}; + for (const auto& test : cases) { + SCOPED_TRACE(test.length); + for (size_t offset = 0; offset < kAlignment; ++offset) { + SCOPED_TRACE(offset); + alignas(uint64_t) std::array input{}; + std::iota(input.begin() + offset, input.begin() + offset + test.length, uint8_t{0}); + EXPECT_EQ(static_cast( + BucketUtils::HashBytes({input.data() + offset, test.length})), + test.hash); + } + } +} + +TEST(BucketUtilsTest, HashLongMatchesCanonicalBytes) { + constexpr std::array values = {0, + 34, + -1, + std::numeric_limits::min(), + std::numeric_limits::max(), + std::numeric_limits::min(), + std::numeric_limits::max(), + 0x0123456789abcdefLL, + static_cast(0xfedcba9876543210ULL)}; + for (const auto value : values) { + SCOPED_TRACE(value); + std::array bytes{}; + for (size_t i = 0; i < bytes.size(); ++i) { + bytes[i] = static_cast(static_cast(value) >> (i * 8)); + } + EXPECT_EQ(BucketUtils::HashLong(value), BucketUtils::HashBytes(bytes)); + } +} + TEST(BucketUtilsTest, BucketTimestampNanosMatchesMicros) { constexpr int32_t kNumBuckets = 1000; const auto ts_micros = TemporalTestHelper::CreateTimestamp({.year = 2017, diff --git a/src/iceberg/test/location_provider_test.cc b/src/iceberg/test/location_provider_test.cc index c78eb588e..268199b3b 100644 --- a/src/iceberg/test/location_provider_test.cc +++ b/src/iceberg/test/location_provider_test.cc @@ -144,6 +144,10 @@ TEST_F(LocationProviderTest, HashInjection) { auto location_d = provider->NewDataLocation("d"); EXPECT_THAT(location_d, testing::EndsWith("/data/1001/0001/0100/01110011/d")); + + auto location_file = provider->NewDataLocation("test.parquet"); + EXPECT_THAT(location_file, + testing::EndsWith("/data/0110/1010/0011/11101000/test.parquet")); } } // namespace iceberg diff --git a/src/iceberg/util/bucket_util.cc b/src/iceberg/util/bucket_util.cc index 23a4d61cd..5e09c033d 100644 --- a/src/iceberg/util/bucket_util.cc +++ b/src/iceberg/util/bucket_util.cc @@ -25,7 +25,6 @@ #include "iceberg/exception.h" #include "iceberg/expression/literal.h" -#include "iceberg/util/endian.h" #include "iceberg/util/murmurhash3_internal.h" #include "iceberg/util/temporal_util.h" @@ -33,10 +32,8 @@ namespace iceberg { namespace { -/// MurmurHash3_x86_32 takes a signed 32-bit length. Iceberg's bucket transform is -/// defined on byte sequences of at most 2^31 - 1 bytes (Java arrays are int-indexed), -/// and longer inputs would wrap the length negative and make the hash loop read far -/// outside the buffer, so they are rejected instead. +// Keep bucket inputs compatible with Java's signed 32-bit array lengths. +// The underlying MurmurHash3 implementation accepts size_t lengths. constexpr size_t kMaxHashBytes = static_cast(std::numeric_limits::max()); /// \brief Returns the number of bytes `HashLiteral` will hash for `literal`. @@ -141,16 +138,11 @@ int32_t BucketUtils::HashBytes(std::span bytes) { "BucketUtils::HashBytes: input length {} exceeds the maximum supported " "length {}", bytes.size(), kMaxHashBytes); - int32_t hash_value = 0; - MurmurHash3_x86_32(bytes.data(), static_cast(bytes.size()), 0, &hash_value); - return hash_value; + return static_cast(MurmurHash3_x86_32(bytes.data(), bytes.size(), 0)); } int32_t BucketUtils::HashLong(int64_t value) { - int32_t hash_value = 0; - value = ToLittleEndian(value); - MurmurHash3_x86_32(&value, sizeof(int64_t), 0, &hash_value); - return hash_value; + return static_cast(MurmurHash3_x86_32_Long(static_cast(value), 0)); } #define DISPATCH_HASH_LITERAL(TYPE_ID) \ diff --git a/src/iceberg/util/bucket_util.h b/src/iceberg/util/bucket_util.h index 07e3f47f3..0ae97178d 100644 --- a/src/iceberg/util/bucket_util.h +++ b/src/iceberg/util/bucket_util.h @@ -51,17 +51,17 @@ class ICEBERG_EXPORT BucketUtils { /// \brief Hash a byte array using MurmurHash3 and return a 32-bit hash value. /// \param bytes The input byte array to hash. /// \return A 32-bit hash value. - /// \throws IcebergError if `bytes.size()` exceeds `INT32_MAX`. MurmurHash3 takes a - /// signed 32-bit length, and the bucket transform is only defined for byte - /// sequences up to that size, so longer inputs are rejected rather than - /// truncated. Java cannot produce such a value because its arrays are - /// int-indexed, so nothing iceberg-java could hash is rejected here. + /// \throws IcebergError if `bytes.size()` exceeds `INT32_MAX`. This limit keeps + /// bucket inputs compatible with Java's int-indexed arrays; the underlying + /// MurmurHash3 implementation accepts `size_t` lengths. static int32_t HashBytes(std::span bytes); /// \brief Compute the bucket index for a given literal and number of buckets. /// \param literal The input literal to hash. /// \param num_buckets The number of buckets to hash into. /// \return (murmur3_x86_32_hash(literal) & Integer.MAX_VALUE) % num_buckets + /// \retval InvalidArgument if num_buckets is not positive or the literal's byte + /// length exceeds `INT32_MAX`; see HashBytes. static Result BucketIndex(const Literal& literal, int32_t num_buckets); }; diff --git a/src/iceberg/util/murmurhash3_internal.cc b/src/iceberg/util/murmurhash3_internal.cc index 14cedabf4..202cf2dc6 100644 --- a/src/iceberg/util/murmurhash3_internal.cc +++ b/src/iceberg/util/murmurhash3_internal.cc @@ -2,443 +2,75 @@ // MurmurHash3 was written by Austin Appleby, and is placed in the public // domain. The author hereby disclaims copyright to this source code. -// Note - The x86 and x64 versions do _not_ produce the same results, as the -// algorithms are optimized for their respective platforms. You can still -// compile and run any of them on any platform, but your performance with the -// non-native version will be less than optimal. - #include "iceberg/util/murmurhash3_internal.h" -//----------------------------------------------------------------------------- -// Platform-specific functions and macros - -// Microsoft Visual Studio - -#if defined(_MSC_VER) - -# define FORCE_INLINE __forceinline - -# include - -# define ROTL32(x, y) _rotl(x, y) -# define ROTL64(x, y) _rotl64(x, y) - -# define BIG_CONSTANT(x) (x) +#include -// Other compilers +#include "iceberg/util/endian.h" -#else // defined(_MSC_VER) - -# define FORCE_INLINE inline __attribute__((always_inline)) - -inline uint32_t rotl32(uint32_t x, int8_t r) { return (x << r) | (x >> (32 - r)); } - -inline uint64_t rotl64(uint64_t x, int8_t r) { return (x << r) | (x >> (64 - r)); } - -# define ROTL32(x, y) rotl32(x, y) -# define ROTL64(x, y) rotl64(x, y) - -# define BIG_CONSTANT(x) (x##LLU) - -#endif // !defined(_MSC_VER) - -// NOLINTBEGIN +// Adapted from the SMHasher MurmurHash3 reference implementation: +// https://github.com/aappleby/smhasher/blob/07bb4de10a63e8cc2e1724865454eba635742383/src/MurmurHash3.cpp namespace iceberg { namespace { -//----------------------------------------------------------------------------- -// Block read - if your platform needs to do endian-swapping or can only -// handle aligned reads, do the conversion here -FORCE_INLINE uint32_t getblock32(const uint32_t* p, int i) { return p[i]; } - -FORCE_INLINE uint64_t getblock64(const uint64_t* p, int i) { return p[i]; } - -//----------------------------------------------------------------------------- -// Finalization mix - force all bits of a hash block to avalanche - -FORCE_INLINE uint32_t fmix32(uint32_t h) { - h ^= h >> 16; - h *= 0x85ebca6b; - h ^= h >> 13; - h *= 0xc2b2ae35; - h ^= h >> 16; - - return h; +uint32_t MixBlock(uint32_t block) { + constexpr uint32_t kC1 = 0xcc9e2d51; + constexpr uint32_t kC2 = 0x1b873593; + block *= kC1; + block = std::rotl(block, 15); + return block * kC2; } -//---------- - -FORCE_INLINE uint64_t fmix64(uint64_t k) { - k ^= k >> 33; - k *= BIG_CONSTANT(0xff51afd7ed558ccd); - k ^= k >> 33; - k *= BIG_CONSTANT(0xc4ceb9fe1a85ec53); - k ^= k >> 33; +uint32_t MixHash(uint32_t hash, uint32_t block) { + hash ^= MixBlock(block); + return std::rotl(hash, 13) * 5 + 0xe6546b64; +} - return k; +uint32_t Finalize(uint32_t hash, size_t len) { + hash ^= static_cast(len); + hash ^= hash >> 16; + hash *= 0x85ebca6b; + hash ^= hash >> 13; + hash *= 0xc2b2ae35; + hash ^= hash >> 16; + return hash; } } // namespace -//----------------------------------------------------------------------------- - -void MurmurHash3_x86_32(const void* key, int len, uint32_t seed, void* out) { - const uint8_t* data = (const uint8_t*)key; - const int nblocks = len / 4; - - uint32_t h1 = seed; - - const uint32_t c1 = 0xcc9e2d51; - const uint32_t c2 = 0x1b873593; - - //---------- - // body +uint32_t MurmurHash3_x86_32(const void* key, size_t len, uint32_t seed) { + // Process the full size_t length; only the final 32-bit length mix may truncate it. + const auto* data = static_cast(key); + const size_t nblocks = len / sizeof(uint32_t); + uint32_t hash = seed; - const uint32_t* blocks = (const uint32_t*)(data + nblocks * 4); - - for (int i = -nblocks; i; i++) { - uint32_t k1 = getblock32(blocks, i); - - k1 *= c1; - k1 = ROTL32(k1, 15); - k1 *= c2; - - h1 ^= k1; - h1 = ROTL32(h1, 13); - h1 = h1 * 5 + 0xe6546b64; + for (size_t i = 0; i < nblocks; ++i) { + hash = MixHash(hash, ReadLittleEndian(data)); + data += sizeof(uint32_t); } - //---------- - // tail - - const uint8_t* tail = (const uint8_t*)(data + nblocks * 4); - - uint32_t k1 = 0; - + uint32_t tail = 0; switch (len & 3) { case 3: - k1 ^= tail[2] << 16; + tail ^= static_cast(data[2]) << 16; [[fallthrough]]; case 2: - k1 ^= tail[1] << 8; + tail ^= static_cast(data[1]) << 8; [[fallthrough]]; case 1: - k1 ^= tail[0]; - k1 *= c1; - k1 = ROTL32(k1, 15); - k1 *= c2; - h1 ^= k1; - }; - - //---------- - // finalization - - h1 ^= len; - - h1 = fmix32(h1); - - *(uint32_t*)out = h1; -} - -//----------------------------------------------------------------------------- - -void MurmurHash3_x86_128(const void* key, const int len, uint32_t seed, void* out) { - const uint8_t* data = (const uint8_t*)key; - const int nblocks = len / 16; - - uint32_t h1 = seed; - uint32_t h2 = seed; - uint32_t h3 = seed; - uint32_t h4 = seed; - - const uint32_t c1 = 0x239b961b; - const uint32_t c2 = 0xab0e9789; - const uint32_t c3 = 0x38b34ae5; - const uint32_t c4 = 0xa1e38b93; - - //---------- - // body - - const uint32_t* blocks = (const uint32_t*)(data + nblocks * 16); - - for (int i = -nblocks; i; i++) { - uint32_t k1 = getblock32(blocks, i * 4 + 0); - uint32_t k2 = getblock32(blocks, i * 4 + 1); - uint32_t k3 = getblock32(blocks, i * 4 + 2); - uint32_t k4 = getblock32(blocks, i * 4 + 3); - - k1 *= c1; - k1 = ROTL32(k1, 15); - k1 *= c2; - h1 ^= k1; - - h1 = ROTL32(h1, 19); - h1 += h2; - h1 = h1 * 5 + 0x561ccd1b; - - k2 *= c2; - k2 = ROTL32(k2, 16); - k2 *= c3; - h2 ^= k2; - - h2 = ROTL32(h2, 17); - h2 += h3; - h2 = h2 * 5 + 0x0bcaa747; - - k3 *= c3; - k3 = ROTL32(k3, 17); - k3 *= c4; - h3 ^= k3; - - h3 = ROTL32(h3, 15); - h3 += h4; - h3 = h3 * 5 + 0x96cd1c35; - - k4 *= c4; - k4 = ROTL32(k4, 18); - k4 *= c1; - h4 ^= k4; - - h4 = ROTL32(h4, 13); - h4 += h1; - h4 = h4 * 5 + 0x32ac3b17; + tail ^= data[0]; } + hash ^= MixBlock(tail); - //---------- - // tail - - const uint8_t* tail = (const uint8_t*)(data + nblocks * 16); - - uint32_t k1 = 0; - uint32_t k2 = 0; - uint32_t k3 = 0; - uint32_t k4 = 0; - - switch (len & 15) { - case 15: - k4 ^= tail[14] << 16; - [[fallthrough]]; - case 14: - k4 ^= tail[13] << 8; - [[fallthrough]]; - case 13: - k4 ^= tail[12] << 0; - k4 *= c4; - k4 = ROTL32(k4, 18); - k4 *= c1; - h4 ^= k4; - [[fallthrough]]; - - case 12: - k3 ^= tail[11] << 24; - [[fallthrough]]; - case 11: - k3 ^= tail[10] << 16; - [[fallthrough]]; - case 10: - k3 ^= tail[9] << 8; - [[fallthrough]]; - case 9: - k3 ^= tail[8] << 0; - k3 *= c3; - k3 = ROTL32(k3, 17); - k3 *= c4; - h3 ^= k3; - [[fallthrough]]; - - case 8: - k2 ^= tail[7] << 24; - [[fallthrough]]; - case 7: - k2 ^= tail[6] << 16; - [[fallthrough]]; - case 6: - k2 ^= tail[5] << 8; - [[fallthrough]]; - case 5: - k2 ^= tail[4] << 0; - k2 *= c2; - k2 = ROTL32(k2, 16); - k2 *= c3; - h2 ^= k2; - [[fallthrough]]; - - case 4: - k1 ^= tail[3] << 24; - [[fallthrough]]; - case 3: - k1 ^= tail[2] << 16; - [[fallthrough]]; - case 2: - k1 ^= tail[1] << 8; - [[fallthrough]]; - case 1: - k1 ^= tail[0] << 0; - k1 *= c1; - k1 = ROTL32(k1, 15); - k1 *= c2; - h1 ^= k1; - }; - - //---------- - // finalization - - h1 ^= len; - h2 ^= len; - h3 ^= len; - h4 ^= len; - - h1 += h2; - h1 += h3; - h1 += h4; - h2 += h1; - h3 += h1; - h4 += h1; - - h1 = fmix32(h1); - h2 = fmix32(h2); - h3 = fmix32(h3); - h4 = fmix32(h4); - - h1 += h2; - h1 += h3; - h1 += h4; - h2 += h1; - h3 += h1; - h4 += h1; - - ((uint32_t*)out)[0] = h1; - ((uint32_t*)out)[1] = h2; - ((uint32_t*)out)[2] = h3; - ((uint32_t*)out)[3] = h4; + return Finalize(hash, len); } -//----------------------------------------------------------------------------- - -void MurmurHash3_x64_128(const void* key, const int len, const uint32_t seed, void* out) { - const uint8_t* data = (const uint8_t*)key; - const int nblocks = len / 16; - - uint64_t h1 = seed; - uint64_t h2 = seed; - - const uint64_t c1 = BIG_CONSTANT(0x87c37b91114253d5); - const uint64_t c2 = BIG_CONSTANT(0x4cf5ad432745937f); - - //---------- - // body - - const uint64_t* blocks = (const uint64_t*)(data); - - for (int i = 0; i < nblocks; i++) { - uint64_t k1 = getblock64(blocks, i * 2 + 0); - uint64_t k2 = getblock64(blocks, i * 2 + 1); - - k1 *= c1; - k1 = ROTL64(k1, 31); - k1 *= c2; - h1 ^= k1; - - h1 = ROTL64(h1, 27); - h1 += h2; - h1 = h1 * 5 + 0x52dce729; - - k2 *= c2; - k2 = ROTL64(k2, 33); - k2 *= c1; - h2 ^= k2; - - h2 = ROTL64(h2, 31); - h2 += h1; - h2 = h2 * 5 + 0x38495ab5; - } - - //---------- - // tail - - const uint8_t* tail = (const uint8_t*)(data + nblocks * 16); - - uint64_t k1 = 0; - uint64_t k2 = 0; - - switch (len & 15) { - case 15: - k2 ^= ((uint64_t)tail[14]) << 48; - [[fallthrough]]; - case 14: - k2 ^= ((uint64_t)tail[13]) << 40; - [[fallthrough]]; - case 13: - k2 ^= ((uint64_t)tail[12]) << 32; - [[fallthrough]]; - case 12: - k2 ^= ((uint64_t)tail[11]) << 24; - [[fallthrough]]; - case 11: - k2 ^= ((uint64_t)tail[10]) << 16; - [[fallthrough]]; - case 10: - k2 ^= ((uint64_t)tail[9]) << 8; - [[fallthrough]]; - case 9: - k2 ^= ((uint64_t)tail[8]) << 0; - k2 *= c2; - k2 = ROTL64(k2, 33); - k2 *= c1; - h2 ^= k2; - [[fallthrough]]; - - case 8: - k1 ^= ((uint64_t)tail[7]) << 56; - [[fallthrough]]; - case 7: - k1 ^= ((uint64_t)tail[6]) << 48; - [[fallthrough]]; - case 6: - k1 ^= ((uint64_t)tail[5]) << 40; - [[fallthrough]]; - case 5: - k1 ^= ((uint64_t)tail[4]) << 32; - [[fallthrough]]; - case 4: - k1 ^= ((uint64_t)tail[3]) << 24; - [[fallthrough]]; - case 3: - k1 ^= ((uint64_t)tail[2]) << 16; - [[fallthrough]]; - case 2: - k1 ^= ((uint64_t)tail[1]) << 8; - [[fallthrough]]; - case 1: - k1 ^= ((uint64_t)tail[0]) << 0; - k1 *= c1; - k1 = ROTL64(k1, 31); - k1 *= c2; - h1 ^= k1; - }; - - //---------- - // finalization - - h1 ^= len; - h2 ^= len; - - h1 += h2; - h2 += h1; - - h1 = fmix64(h1); - h2 = fmix64(h2); - - h1 += h2; - h2 += h1; - - ((uint64_t*)out)[0] = h1; - ((uint64_t*)out)[1] = h2; +uint32_t MurmurHash3_x86_32_Long(uint64_t value, uint32_t seed) { + auto hash = MixHash(seed, static_cast(value)); + hash = MixHash(hash, static_cast(value >> 32)); + return Finalize(hash, sizeof(value)); } -//----------------------------------------------------------------------------- } // namespace iceberg - -// NOLINTEND diff --git a/src/iceberg/util/murmurhash3_internal.h b/src/iceberg/util/murmurhash3_internal.h index 5b71f930a..3ec41ea6d 100644 --- a/src/iceberg/util/murmurhash3_internal.h +++ b/src/iceberg/util/murmurhash3_internal.h @@ -4,35 +4,16 @@ #pragma once -//----------------------------------------------------------------------------- -// Platform-specific functions and macros - -// Microsoft Visual Studio - -#if defined(_MSC_VER) && (_MSC_VER < 1600) - -typedef unsigned char uint8_t; -typedef unsigned int uint32_t; -typedef unsigned __int64 uint64_t; - -// Other compilers - -#else // defined(_MSC_VER) - -# include - -#endif // !defined(_MSC_VER) +#include +#include namespace iceberg { -//----------------------------------------------------------------------------- - -void MurmurHash3_x86_32(const void* key, int len, uint32_t seed, void* out); +// MurmurHash3 reads blocks in little-endian order, including unaligned input, +// and returns a numeric hash rather than a serialized byte sequence. +uint32_t MurmurHash3_x86_32(const void* key, size_t len, uint32_t seed); -void MurmurHash3_x86_128(const void* key, int len, uint32_t seed, void* out); - -void MurmurHash3_x64_128(const void* key, int len, uint32_t seed, void* out); - -//----------------------------------------------------------------------------- +// Hash a 64-bit value as two little-endian blocks, without serializing it to memory. +uint32_t MurmurHash3_x86_32_Long(uint64_t value, uint32_t seed); } // namespace iceberg