Follow up changes to the type hash function (#946)

I missed a few comments from the previous PR (#943). This change addresses the remaining comments.

production/catalog/src/ddl_executor.cpp

@@ -791,8 +791,8 @@ uint32_t generate_table_type(const string& db_name, const string& table_name)
     ASSERT_PRECONDITION(db_name.length() <= std::numeric_limits<int>::max(), "The DB name is too long.");
     ASSERT_PRECONDITION(table_name.length() <= std::numeric_limits<int>::max(), "The table name is too long.");
 
-    return murmurhash3_x86_32(table_name.data(), static_cast<int>(table_name.length()))
-        ^ (murmurhash3_x86_32(db_name.data(), static_cast<int>(db_name.length())) << 1);
+    return hash::murmur3_32(table_name.data(), static_cast<int>(table_name.length()))
+        ^ (hash::murmur3_32(db_name.data(), static_cast<int>(db_name.length())) << 1);
 }
 
 gaia_id_t ddl_executor_t::create_table_impl(

production/inc/gaia_internal/common/hash.hpp

@@ -7,11 +7,41 @@
 #include <cstdint>
 #include <cstring>
 
-// Adapted from the public domain murmur3 hash implementation at:
+#include "gaia_internal/common/retail_assert.hpp"
+
+namespace gaia
+{
+namespace common
+{
+namespace hash
+{
+
+// Replacement of `std::rotl` before C++ 20 from the post below:
+// https://blog.regehr.org/archives/1063
+inline uint32_t rotl32(uint32_t x, uint32_t n)
+{
+    // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers)
+    ASSERT_PRECONDITION(n < 32, "Out of range rotation number.");
+    // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers)
+    return (x << n) | (x >> (-n & 31));
+}
+
+// Compute murmur3 32 bit hash for the key. Adapted from the public domain
+// murmur3 hash implementation at:
 // https://github.com/aappleby/smhasher/blob/master/src/MurmurHash3.cpp
-uint32_t murmurhash3_x86_32(const void* key, int len)
+//
+// We made the following changes to the original implementation.
+// - Use the key length as the seed.
+// - Use `std::memcpy` for block read.
+// - Return `uint32_t` directly to avoid block write.
+// - Change C-style cast to C++ cast and switch to `auto` type.
+// - Code format change in various places to adhering to Gaia coding guideline.
+//
+// Warning: murmur3 is not a cryptographic hash function and should not be used
+// in places where security is a concern.
+uint32_t murmur3_32(const void* key, int len)
 {
-    const auto* data = static_cast<const uint8_t*>(key);
+    auto data = static_cast<const uint8_t*>(key);
     const int nblocks = len / 4;
 
     uint32_t h1 = len;
@@ -22,7 +52,7 @@ uint32_t murmurhash3_x86_32(const void* key, int len)
     //----------
     // body
 
-    const auto* blocks = reinterpret_cast<const uint32_t*>(data + nblocks * 4);
+    auto blocks = reinterpret_cast<const uint32_t*>(data + nblocks * 4);
 
     for (int i = -nblocks; i; i++)
     {
@@ -31,20 +61,20 @@ uint32_t murmurhash3_x86_32(const void* key, int len)
 
         k1 *= c1;
         // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers)
-        k1 = (k1 << 15) | (k1 >> (32 - 15));
+        k1 = rotl32(k1, 15);
         k1 *= c2;
 
         h1 ^= k1;
         // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers)
-        h1 = (k1 << 13) | (k1 >> (32 - 13));
+        h1 = rotl32(h1, 13);
         // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers)
         h1 = h1 * 5 + 0xe6546b64;
     }
 
     //----------
     // tail
 
-    const auto* tail = static_cast<const uint8_t*>(data + nblocks * 4);
+    auto tail = static_cast<const uint8_t*>(data + nblocks * 4);
 
     uint32_t k1 = 0;
 
@@ -60,7 +90,7 @@ uint32_t murmurhash3_x86_32(const void* key, int len)
         k1 ^= tail[0];
         k1 *= c1;
         // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers)
-        k1 = (k1 << 15) | (k1 >> (32 - 15));
+        k1 = rotl32(k1, 15);
         k1 *= c2;
         h1 ^= k1;
     };
@@ -83,3 +113,7 @@ uint32_t murmurhash3_x86_32(const void* key, int len)
 
     return h1;
 }
+
+} // namespace hash
+} // namespace common
+} // namespace gaia