feat: Add support for evaluating Iceberg partition transforms #15440

PingLiuPing · 2025-11-07T15:20:07Z

Introduce infrastructure for evaluating Iceberg partition transforms.

TransformExprBuilder converts Iceberg partition specifications into Velox expressions
TransformEvaluator evaluates multiple transform expressions in a single pass using compiled ExprSet.

netlify · 2025-11-07T15:20:13Z

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	`fe7e347`
🔍 Latest deploy log	https://app.netlify.com/projects/meta-velox/deploys/6911e45d50d45900096cac03

PingLiuPing · 2025-11-07T15:30:09Z

@mbasmanova This PR continues from #15035 (review).
In next PR, I plan to include IcebergPartitionIdGenerator and do some refactor on Hive PartitionIdGenerator, I roughly count the lines which should be also around 700 lines. After that, all the preliminary work should be complete, and I’ll submit another PR to integrate them with IcebergDataSink.

mbasmanova · 2025-11-07T17:18:36Z

Ci is red

mbasmanova · 2025-11-07T17:20:22Z

velox/connectors/hive/iceberg/IcebergPartitionPath.h

+
+/// Converts a partition value to its string representation for use in
+/// partition directory path. The format follows the Iceberg specification
+/// for partition path encoding.


Do you have a link to the spec? Would be nice to add here.

Thanks for the comment.
I cannot find an explicit document about this. I will revise the comment.
I followed the Iceberg java code to implement this.

mbasmanova · 2025-11-07T17:22:50Z

velox/connectors/hive/iceberg/IcebergPartitionPath.h

+  explicit IcebergPartitionPath(TransformType transformType)
+      : transformType_(transformType) {}
+
+  ~IcebergPartitionPath() override = default;


mbasmanova · 2025-11-07T17:23:32Z

velox/connectors/hive/iceberg/IcebergPartitionPath.h

+
+  using HivePartitionUtil::toPartitionString;
+
+  std::string toPartitionString(int32_t value, const TypePtr& type)


perhaps, document the format of the result

mbasmanova · 2025-11-07T17:25:13Z

velox/connectors/hive/iceberg/IcebergPartitionPath.h

+/// Converts a partition value to its string representation for use in
+/// partition directory path. The format follows the Iceberg specification
+/// for partition path encoding.
+class IcebergPartitionPath : public HivePartitionUtil {


Would it be possible to extract this class into a separate PR and add a test?

Thank you for reviewing.

I will separate this to a new PR.

mbasmanova · 2025-11-07T17:25:59Z

velox/connectors/hive/iceberg/TransformExprBuilder.h

+  /// type, and optional parameter (e.g., bucket count, truncate width).
+  /// @param inputFieldName Name of the source column in the input RowVector.
+  /// @return Typed expression representing the transform.
+  static core::TypedExprPtr toExpression(


Do we need to expose this API?

Thanks. I moved it to anonymous namespace.

mbasmanova · 2025-11-07T17:27:32Z

velox/connectors/hive/iceberg/TransformEvaluator.cpp

+
+namespace {
+
+constexpr char const* kIcebergFunctionPrefix{"iceberg_"};


move this next to its only use

mbasmanova · 2025-11-07T17:28:21Z

velox/connectors/hive/iceberg/tests/TransformTest.cpp

+class TransformTest : public test::IcebergTestBase {
+ protected:
+  template <typename T>
+  VectorPtr createFlatVector(


Why not use VectorMaker or helper methods from VectorTestBase?

Thanks. Changed to VectorMaker

mbasmanova · 2025-11-07T17:29:43Z

velox/connectors/hive/iceberg/tests/TransformTest.cpp

+
+    // Create expected vector and verify results.
+    auto expectedVector = createFlatVector<OUT>(expectedValues);
+    ASSERT_EQ(resultVector[0]->size(), expectedValues.size());


use assertEqualVectors or similar from VectorTestBase.h

PingLiuPing · 2025-11-07T19:48:17Z

velox/connectors/hive/iceberg/PartitionSpec.h

        case TransformType::kTruncate:
          return type;
      }
+      VELOX_UNREACHABLE("Unknown transform type");


@mbasmanova Have to add this to fix the building break on Linux. Macos is fine without this change.

mbasmanova · 2025-11-07T21:20:29Z

velox/connectors/hive/iceberg/tests/TransformTest.cpp

+      const std::vector<std::optional<IN>>& inputValues,
+      const std::vector<std::optional<OUT>>& expectedValues,


Can we replace these arguments with RowVectorPtr and use makeFlatVector and similar in the callers?

Thanks. Just updated.

mbasmanova · 2025-11-07T22:54:04Z

velox/connectors/hive/iceberg/tests/TransformTest.cpp

+ protected:
+  void testTransform(
+      const IcebergPartitionSpec::Field& field,
+      const RowVectorPtr& inputRowVector,


nit: inputRowVector -> input, expectedRowVector -> expected

mbasmanova · 2025-11-07T22:54:17Z

velox/connectors/hive/iceberg/tests/TransformTest.cpp

+    auto transformExprs = TransformExprBuilder::toExpressions(
+        spec,
+        std::vector<column_index_t>{0},
+        asRowType(inputRowVector->type()));


input->rowType()

mbasmanova · 2025-11-07T22:54:39Z

velox/connectors/hive/iceberg/tests/TransformTest.cpp

+      const RowVectorPtr& inputRowVector,
+      const RowVectorPtr& expectedRowVector) {
+    // Build and evaluate transform expressions.
+    auto spec = std::make_shared<const IcebergPartitionSpec>(


Why do we test single field specs only? Why not test multi-field specs?

Thank you for the comment.
I added those test one by one when I implement the transformation logic one by one, and type by type.
I will refactor it to support test multiple transforms.

mbasmanova · 2025-11-07T22:56:25Z

velox/connectors/hive/iceberg/tests/TransformTest.cpp

+          {makeFlatVector<StringView>(
+              {StringView("\x01\x02\x03", 3), StringView("", 0)},
+              VARBINARY())}),
+      makeRowVector({makeFlatVector<StringView>(


drop StringView or use std::string

makeFlatVector<std::string>({"foo", "bar"})

mbasmanova · 2025-11-10T10:59:22Z

velox/connectors/hive/iceberg/TransformEvaluator.cpp

+
+  // Register Iceberg functions once for expression evaluation.
+  static std::once_flag registerFlag;
+  static constexpr char const* kIcebergFunctionPrefix{"iceberg_"};


iceberg_ prefix is now hard-coded in 2 places: here and in TransformExprBuilder.cpp

Also, It seems better to make these functions available unconditionally, not only after running a CTAS or INSERT INTO query.

I suggest to move the logic for registering these functions to the same place where we register Iceberg connector and add a constant for prefix, which can be reused without hard-coding multiple times.

Thank you for the comment.
Iceberg connector does not exist at the moment. Can I register these functions when registering Hive connector?

Iceberg connector does not exist at the moment.

Hmm... but then, how can it be used? Should we create one or at least add a function to register what's needed for Iceberg?

Currently it depends on HiveConnector::createDataSink to create an IcebergDataSink.

Should we create one or at least add a function to register what's needed for Iceberg?

How about adding a function first in this PR, and adding IcebergConnector in a separate PR?

How about adding a function first in this PR, and adding IcebergConnector in a separate PR?

Sounds good.

mbasmanova · 2025-11-10T11:01:42Z

velox/connectors/hive/iceberg/tests/TransformTest.cpp

+           makeFlatVector<std::string>({("\x01\x02\x03"), ("")}, VARBINARY()),
+           makeFlatVector<Timestamp>(
+               {Timestamp(0, 0), Timestamp(1609459200, 0)})}),
+      makeRowVector(


Let's not copy-paste expected values for identity transform. Let's use 'input'.

mbasmanova · 2025-11-10T11:02:04Z

velox/connectors/hive/iceberg/tests/TransformTest.cpp

+              makeNullableFlatVector<Timestamp>({std::nullopt}),
+              makeNullableFlatVector<Timestamp>({std::nullopt}),
+          }),
+      makeRowVector({


Thanks, this case tested all transforms on null value. Most of the expected vector type is different with input.

mbasmanova · 2025-11-10T11:03:50Z

velox/connectors/hive/iceberg/tests/TransformTest.cpp

+
+TEST_F(TransformTest, multipleTransforms) {
+  const auto& rowType =
+      ROW({"c_int", "c_date", "c_varchar"}, {INTEGER(), DATE(), VARCHAR()});


no need to use custom column names; use c0, c1, c2... and then drop rowType->name() argument from makeRowVector; ditto all other places

mbasmanova

Thanks.

PingLiuPing · 2025-11-10T17:34:56Z

@peterenescu Could you please help to import this PR? Thank you.

meta-codesync · 2025-11-10T18:21:48Z

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

meta-codesync · 2025-11-10T23:26:29Z

@peterenescu merged this pull request in bef78ba.

…facebookincubator#15440)" This reverts commit bef78ba.

…facebookincubator#15440)" This reverts commit bef78ba. Alchemy-item: (ID = 854) Iceberg staging hub commit 2/14 - ecf692f

facebook-github-bot · 2025-11-12T05:02:23Z

This pull request has been reverted by 4056e41.

PingLiuPing requested a review from majetideepak as a code owner November 7, 2025 15:20

meta-cla 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 Nov 7, 2025

PingLiuPing requested review from mbasmanova and removed request for majetideepak November 7, 2025 15:20

mbasmanova reviewed Nov 7, 2025

View reviewed changes

PingLiuPing force-pushed the lp_iceberg_transform_eval branch from ff2e9e0 to ae18463 Compare November 7, 2025 19:47

PingLiuPing commented Nov 7, 2025

View reviewed changes

mbasmanova reviewed Nov 7, 2025

View reviewed changes

PingLiuPing force-pushed the lp_iceberg_transform_eval branch from ae18463 to d007247 Compare November 7, 2025 22:15

mbasmanova reviewed Nov 7, 2025

View reviewed changes

PingLiuPing force-pushed the lp_iceberg_transform_eval branch from d007247 to 8047d9b Compare November 8, 2025 20:26

mbasmanova reviewed Nov 10, 2025

View reviewed changes

Add iceberg partition transform evaluator and utility

fe7e347

PingLiuPing force-pushed the lp_iceberg_transform_eval branch from 8047d9b to fe7e347 Compare November 10, 2025 13:10

mbasmanova approved these changes Nov 10, 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 Nov 10, 2025

mbasmanova changed the title ~~feat: Add iceberg partition transform evaluator and utility~~ feat: Add support for evaluating Iceberg partition transforms Nov 10, 2025

meta-codesync bot closed this in bef78ba Nov 10, 2025

facebook-github-bot added the Merged label Nov 10, 2025

PingLiuPing added a commit to IBM/velox that referenced this pull request Nov 11, 2025

Revert "feat: Add support for evaluating Iceberg partition transforms (…

ecf692f

…facebookincubator#15440)" This reverts commit bef78ba.

amitkdutta mentioned this pull request Nov 12, 2025

chore(ci): Advance velox prestodb/presto#26584

Merged

facebook-github-bot added the Reverted label Nov 12, 2025

PingLiuPing mentioned this pull request Nov 12, 2025

feat: Add support for evaluating Iceberg partition transforms #15477

Closed

PingLiuPing mentioned this pull request Nov 20, 2025

refactor: Add Iceberg connector #15581

Closed


		using HivePartitionUtil::toPartitionString;

		std::string toPartitionString(int32_t value, const TypePtr& type)


		namespace {

		constexpr char const* kIcebergFunctionPrefix{"iceberg_"};

		const std::vector<std::optional<IN>>& inputValues,
		const std::vector<std::optional<OUT>>& expectedValues,

feat: Add support for evaluating Iceberg partition transforms #15440

feat: Add support for evaluating Iceberg partition transforms #15440

Uh oh!

Conversation

PingLiuPing commented Nov 7, 2025 • edited by mbasmanova Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Nov 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!

PingLiuPing commented Nov 7, 2025

Uh oh!

mbasmanova commented Nov 7, 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!

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!

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

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!

PingLiuPing Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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

PingLiuPing commented Nov 7, 2025 •

edited by mbasmanova

Loading

netlify bot commented Nov 7, 2025 •

edited

Loading

mbasmanova Nov 7, 2025 •

edited

Loading

PingLiuPing Nov 8, 2025 •

edited

Loading