Make partitionValues serializable for IcebergSplit

[chenjian2664]

Description

The ConnectorSplit serves as the data carrier between the coordinator and workers, and therefore should be fully serializable by design. Making partitionValues serializable is the first step toward correcting the current implementation.

Additional context and related issues

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

## Section
* Fix some things. ({issue}`issuenumber`)

[starburstdata-automation]

Started benchmark workflow for this PR with test type = iceberg/sf1000_parquet_part.

Building Trino finished with status: success
Benchmark finished with status: success
Comparing results to the static baseline values, follow above workflow link for more details/logs.
Status message: NO Regression found.
Benchmark Comparison to the closest run from Master: Report

[chenjian2664]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[chenjian2664]

@raunaqmorarka PTAL

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request refactors partition value representation in the Iceberg plugin, replacing JSON-based serialization with direct Trino Block objects. Changes include replacing Optional<List<Object>> partitionValues and String partitionDataJson in IcebergSplit with a single List<Block> partitionValues field. The partitionStructFields metadata is moved from IcebergTablePartitioning to IcebergPartitioningHandle. A new factory method PartitionData.fromBlocks() is introduced to construct partition data directly from blocks, and partition value handling is updated across IcebergSplitSource, IcebergBucketFunction, and IcebergPageSourceProvider to work with the new block-based representation.

Possibly related PRs

• [Iceberg v3] Nanosecond precision timestamps #27835: Extends timestamp and nanos handling in PartitionData and related partition value flows, directly updating the same classes modified in this PR (PartitionData, IcebergSplit, partition handling pipeline).

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.42.0)

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergSplit.java (1)

210-217: ⚠️ Potential issue | 🟡 Minor

Use estimatedSizeOf for partitionValues here.

Line 216 only sums the blocks themselves, so getRetainedSizeInBytes() misses the partitionValues list shell. That under-reports split memory in the same method that otherwise uses SizeOf helpers for containers.

🧮 Proposed fix

-                + partitionValues.stream().map(Block::getRetainedSizeInBytes).mapToInt(Long::intValue).sum()
+                + estimatedSizeOf(partitionValues, Block::getRetainedSizeInBytes)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergSplit.java`
around lines 210 - 217, The getRetainedSizeInBytes method underreports memory
because it sums only the Block sizes and omits the partitionValues list
overhead; in IcebergSplit.getRetainedSizeInBytes replace the current
partitionValues.stream().map(Block::getRetainedSizeInBytes).mapToInt(Long::intValue).sum()
expression with estimatedSizeOf(partitionValues, Block::getRetainedSizeInBytes)
so the list shell plus element sizes are included (use the existing
estimatedSizeOf helper and the partitionValues symbol to locate the change).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In
`@plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergSplit.java`:
- Around line 210-217: The getRetainedSizeInBytes method underreports memory
because it sums only the Block sizes and omits the partitionValues list
overhead; in IcebergSplit.getRetainedSizeInBytes replace the current
partitionValues.stream().map(Block::getRetainedSizeInBytes).mapToInt(Long::intValue).sum()
expression with estimatedSizeOf(partitionValues, Block::getRetainedSizeInBytes)
so the list shell plus element sizes are included (use the existing
estimatedSizeOf helper and the partitionValues symbol to locate the change).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fdb49aef-a257-46fb-9e17-d42d4682935a

📥 Commits

Reviewing files that changed from the base of the PR and between 11bf3c0 and 380cef3.

📒 Files selected for processing (11)

• plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergBucketFunction.java
• plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
• plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergPageSourceProvider.java
• plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergPartitioningHandle.java
• plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergSplit.java
• plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergSplitSource.java
• plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergTablePartitioning.java
• plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/PartitionData.java
• plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/functions/tablechanges/TableChangesFunctionProcessor.java
• plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergNodeLocalDynamicSplitPruning.java
• plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergPageSourceProvider.java

💤 Files with no reviewable changes (2)

• plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/functions/tablechanges/TableChangesFunctionProcessor.java
• plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergPageSourceProvider.java

[raunaqmorarka]

Review comments on partition value serialization changes.

[raunaqmorarka]

lgtm % minor comment