feat(iceberg): Support Iceberg hash #14025

jinchengchenghh · 2025-07-07T03:00:06Z

The iceberg hash use mumur3 hash, which aligns with https://github.com/aappleby/smhasher/blob/master/src/MurmurHash3.cpp, firstly, process every 4 bytes as a chunk, then process remaining bytes by XOR, sparksql also uses this hash algorithm but is different with processing remaining bytes, which combine the remaining bytes. Extract the common function hashInt64 to functions/lib.

This class will be used for iceberg bucket transform and bucket function.
The iceberg mumur3 hash should be strictly with java implementation, then write by iceberg could read with iceberg Java, and the function call can also get the correct result.
The iceberg utility lib velox_functions_iceberg_hash will be linked by iceberg connector write to do partition transform. #13874

netlify · 2025-07-07T03:00:11Z

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	`f355240`
🔍 Latest deploy log	https://app.netlify.com/projects/meta-velox/deploys/688c54679b1dc4000892f872

velox/functions/iceberg/Murmur3Hash32.cpp

velox/functions/iceberg/CMakeLists.txt

velox/functions/iceberg/Murmur3Hash32.cpp

velox/functions/iceberg/Murmur3Hash32.h

velox/functions/lib/tests/Murmur3Hash32Test.cpp

jinchengchenghh · 2025-07-25T08:50:27Z

Could you help review this PR? Thanks! @mbasmanova

mbasmanova

Some comments.

mbasmanova · 2025-07-25T09:55:16Z

velox/functions/iceberg/CMakeLists.txt

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+velox_add_library(velox_functions_iceberg_util Murmur3Hash32.cpp)


velox_functions_iceberg_util

Let's match the file path: velox_functions_iceberg

I use the same format as https://github.com/facebookincubator/velox/blob/main/velox/functions/lib/CMakeLists.txt#L19

The velox_functions_iceberg_util will also be linked by IcebergConnector for partition transform, velox_functions_iceberg links too much library, so I add a new utility library.

@jinchengchenghh "util" name it too generic and should be avoided if possible. Here, we have only one library / binary in a folder, hence, we can use the folder's path to name the library. No need to append any suffix.

After support the Iceberg function like bucket, https://github.com/facebookincubator/velox/pull/13174/files#diff-588a1233022efcdaf5de5fe7eeb67b699f62e7ece9c3e64130be130091b27423, https://github.com/facebookincubator/velox/pull/13174/files#diff-cee1c2962987fd80218c2475980e18d021179d632f31f9fcdd325f35fb13a733R17, we will have two libraries.

@jinchengchenghh since this library provides a hash function, perhaps, we can name it velox_functions_iceberg_hash.

It will also provide utility function like DateTimeUtil for years transformer.

It will also provide utility function like DateTimeUtil for years transformer.

It is generally not ideal to create utils that contain multiple unrelated functions. It would be better to keep hash-related utilities separate from date-time-related ones.

Yes, updated to velox_functions_iceberg_hash

mbasmanova · 2025-07-25T09:56:59Z

velox/functions/lib/Hash.h

+/// original) to avoid undefined signed integer overflow and sign extension.
+class Murmur3Hash32 {
+ public:
+  /// Hash the lower int , is a fast path of hashBytes.


What does "lower int" refer to here? "is a fast path of hashBytes" - which hashBytes does this refer to?

hashBytes for int64 is same, hashBytes process 4 bytes as a chunk, only processing the remaining bytes is different.

Update the comments

The subclass implements hashBytes, do we need to have a virtual function hashBytes? But then the function cannot be static.

velox/functions/lib/Hash.cpp

velox/functions/lib/Hash.h

velox/functions/iceberg/Murmur3Hash32.h

velox/functions/iceberg/Murmur3Hash32.cpp

velox/functions/lib/Hash.h

jinchengchenghh · 2025-07-28T07:22:54Z

Resolved all the comments, could you help review again? Thanks! @mbasmanova

mbasmanova · 2025-07-28T13:06:00Z

velox/functions/iceberg/tests/Murmur3Hash32Test.cpp

+
+#include <gtest/gtest.h>
+
+using namespace facebook::velox::functions::iceberg;


The test goes into anonymous namespace inside the namespace that contains code being tested.

namespace facebook::velox::functions::iceberg { namespace { TEST(Murmur3Hash32Test, bigint) { ... } }

mbasmanova · 2025-07-28T13:07:33Z

velox/functions/lib/Murmur3Hash32Base.h

+  /// hashBytes.
+  static uint32_t hashInt64(uint64_t input, uint32_t seed);
+
+ protected:


I think all methods here can be public. If you want to have some protected, then hashInt64 also needs to be protected for consistency.

mbasmanova · 2025-07-28T13:08:18Z

velox/functions/lib/tests/Murmur3Hash32BaseTest.cpp

+#include <gtest/gtest.h>
+#include "velox/common/base/tests/GTestUtils.h"
+
+namespace {


As above, the test goes into anonymous namespace inside namespace that contains code being tested.

mbasmanova

@jinchengchenghh Thank you for iterating. Looks good % a few nits.

jinchengchenghh · 2025-07-29T08:09:51Z

Do you have further comments? Thanks! @mbasmanova

mbasmanova

Thanks.

kgpai · 2025-07-31T18:29:10Z

@jinchengchenghh Can you look if the failing tests related to your changes ? Can you also rebase to latest main ? Thanks.

jinchengchenghh · 2025-08-01T05:46:15Z

The failure is not related to this PR, rebased just now, waiting to see if the error exists. @kgpai

jinchengchenghh · 2025-08-01T09:13:38Z

The failure is tracked by #14308, and is not related to this PR

facebook-github-bot · 2025-08-06T16:49:15Z

@kgpai has imported this pull request. If you are a Meta employee, you can view this in D79732785.

facebook-github-bot · 2025-08-06T22:41:18Z

@kgpai has imported this pull request. If you are a Meta employee, you can view this in D79732785.

facebook-github-bot · 2025-08-07T06:44:11Z

@kgpai merged this pull request in a417b2b.

Summary: The iceberg hash use mumur3 hash, which aligns with https://github.com/aappleby/smhasher/blob/master/src/MurmurHash3.cpp, firstly, process every 4 bytes as a chunk, then process remaining bytes by XOR, sparksql also uses this hash algorithm but is different with processing remaining bytes, which combine the remaining bytes. Extract the common function hashInt64 to functions/lib. This class will be used for iceberg bucket transform and bucket function. The iceberg mumur3 hash should be strictly with java implementation, then write by iceberg could read with iceberg Java, and the function call can also get the correct result. The iceberg utility lib `velox_functions_iceberg_hash` will be linked by iceberg connector write to do partition transform. facebookincubator#13874 Pull Request resolved: facebookincubator#14025 Reviewed By: pedroerp Differential Revision: D79732785 Pulled By: kgpai fbshipit-source-id: 6122b94673f015dca5c8484722926709a30fe65e

jinchengchenghh requested review from assignUser and majetideepak as code owners July 7, 2025 03:00

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 7, 2025

jinchengchenghh changed the title ~~Iceberg hash~~ feat(iceberg): Support Iceberg hash Jul 7, 2025

jinchengchenghh force-pushed the iceberg_hash branch from 282f1be to cc51f43 Compare July 7, 2025 14:05

PingLiuPing reviewed Jul 7, 2025

View reviewed changes

mbasmanova reviewed Jul 25, 2025

View reviewed changes

jinchengchenghh force-pushed the iceberg_hash branch from ba51c1a to 9246ce9 Compare July 28, 2025 05:24

mbasmanova reviewed Jul 28, 2025

View reviewed changes

mbasmanova approved these changes Jul 29, 2025

View reviewed changes

mbasmanova added the ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall label Jul 29, 2025

jinchengchenghh added 10 commits August 1, 2025 13:44

support iceberg Hash function

71c3e0e

Move hash to util

3c702fd

refactor iceberg util

1861471

remove folly

77b54d0

address comments

79ba8e0

address comments

463c1a6

fix code style

ea3a9dd

address comments

d748dbf

fix code style

359ef10

Rename the iceberg util library

f355240

jinchengchenghh force-pushed the iceberg_hash branch from 2e2b5a3 to f355240 Compare August 1, 2025 05:45

facebook-github-bot closed this in a417b2b Aug 7, 2025

facebook-github-bot added the Merged label Aug 7, 2025


		#include <gtest/gtest.h>

		using namespace facebook::velox::functions::iceberg;

feat(iceberg): Support Iceberg hash #14025

feat(iceberg): Support Iceberg hash #14025

Uh oh!

Conversation

jinchengchenghh commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for meta-velox canceled.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jinchengchenghh commented Jul 25, 2025

Uh oh!

mbasmanova left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jinchengchenghh commented Jul 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbasmanova left a comment

Choose a reason for hiding this comment

Uh oh!

jinchengchenghh commented Jul 29, 2025

Uh oh!

mbasmanova left a comment

Choose a reason for hiding this comment

Uh oh!

kgpai commented Jul 31, 2025

Uh oh!

jinchengchenghh commented Aug 1, 2025

Uh oh!

jinchengchenghh commented Aug 1, 2025

Uh oh!

facebook-github-bot commented Aug 6, 2025

Uh oh!

facebook-github-bot commented Aug 6, 2025

Uh oh!

facebook-github-bot commented Aug 7, 2025

Uh oh!

jinchengchenghh commented Jul 7, 2025 •

edited

Loading

netlify bot commented Jul 7, 2025 •

edited

Loading