feat: support to read chain data split by Weixin-Xu · Pull Request #387 · alibaba/paimon-cpp

Weixin-Xu · 2026-06-29T07:54:40Z

Purpose

Linked issue: close #385

Support deserializing, serializing, and reading community-format ChainSplit.

This change adds ChainSplit support for the current Java SplitSerializer wire format, including:

recognizing the SplitSerializer magic, serializer version, and CHAIN_SPLIT type
deserializing ChainSplit fields: logical partition, data files, file bucket path mapping, and file branch mapping
serializing ChainSplit back through SplitSerializer without dropping chain metadata
resolving ChainSplit data file paths through per-file bucket path mapping
failing fast when a non-external ChainSplit file is missing bucket path mapping
preserving existing DataSplit, FallbackDataSplit, and IndexedSplit behavior

Tests

Added ChainSplitTest
- deserialize community-format ChainSplit bytes
- deserialize ChainSplit bytes with multiple data files
- verify per-file bucket path resolution
- verify external file path is preserved
- verify missing bucket path mapping returns an error
- verify ChainSplit serialization preserves bucket and branch mappings
- verify malformed ChainSplit bytes return an error

API and Format

This change does not modify table storage format. It updates split protocol support to handle the community Java ChainSplit format.

The read path consumes fileBucketPathMapping for per-file bucket path resolution. It also deserializes and preserves fileBranchMapping for format compatibility; branch-aware read/schema selection is intentionally deferred to a follow-up change.

Documentation

No user-facing documentation update.

Generative AI tooling

Generated-by: OpenAI Codex

CLAassistant · 2026-06-29T07:54:48Z

All committers have signed the CLA.

zjw1111

Thanks for adding ChainDataSplit support! The overall design looks good. A few minor suggestions below.

zjw1111 · 2026-06-30T10:51:47Z

    Result<std::unique_ptr<BatchReader>> ApplyPredicateFilterIfNeeded(
        std::unique_ptr<BatchReader>&& reader, const std::shared_ptr<Predicate>& predicate) const;

+    Result<std::shared_ptr<DataFilePathFactory>> CreateDataFilePathFactory(


Minor: CreateDataFilePathFactory is currently in the public section, but it seems to only be called by subclasses internally. Would it be possible to move it into the protected section just below?

Fixed in the latest revision. CreateDataFilePathFactory is now protected since it is only used by AbstractSplitRead subclasses.

zjw1111 · 2026-06-30T10:51:47Z

+
+    const std::unordered_map<std::string, std::string>& FileBucketPathMapping() const {
+        return file_bucket_path_mapping_;
+    }


I noticed that file_branch_mapping_ is deserialized and stored, but not consumed by any read path in this PR (e.g. ChainDataFilePathFactory only uses file_bucket_path_mapping_). I assume this is reserved for future use (e.g. selecting the correct schema per branch, as the Java side does). Could you add a brief note in the PR description mentioning this is intentionally deferred?

Added a note in the API and Format section to clarify that file_branch_mapping is preserved for ChainDataSplit metadata compatibility, while branch-aware read/schema selection is deferred. The current read path only consumes file_bucket_path_mapping.

lxy-9602 · 2026-06-30T10:52:24Z

+    if (!data_file_path_factory) {
+        PAIMON_ASSIGN_OR_RAISE(data_file_path_factory,
+                               path_factory_->CreateDataFilePathFactory(partition, bucket));
+    }


This design feels a bit too implicit. Why can’t each caller just pass in the correct data_file_path_factory directly?

Also, let’s avoid using default arguments in production code.

Fixed. The callers now construct the appropriate DataFilePathFactory explicitly before delegating:

split-based reads use CreateDataFilePathFactory(data_split), so ChainDataSplit can get the chain-aware factory.

partition/bucket reads create the normal factory with path_factory_->CreateDataFilePathFactory(partition, bucket).

The private helper now requires a DataFilePathFactory argument directly, so there is no nullptr fallback and no default argument.

lxy-9602 · 2026-06-30T10:58:16Z

+                return Status::Invalid(fmt::format("invalid ChainDataSplit byte stream: {}",
+                                                   chain_split.status().ToString()));
+            }
+            return std::static_pointer_cast<Split>(chain_split.value());


Could you please use PAIMON_ASSIGN_OR_RAISE rather than if (!chain_split.ok()).

Fixed. Replaced the manual chain_split.ok() checks with PAIMON_ASSIGN_OR_RAISE in both ChainDataSplit tail parsing paths.

lxy-9602 · 2026-06-30T11:01:23Z

+
+Result<std::shared_ptr<ChainDataSplitImpl>> ReadChainDataSplitTail(
+    const std::shared_ptr<DataSplitImpl>& base_split, DataInputStream* in,
+    const std::shared_ptr<MemoryPool>& pool) {


Could you move the output parameter (in) in to the end of the parameter list?

Fixed. Moved DataInputStream* in to the end of ReadChainDataSplitTail's parameter list and updated both call sites.

lxy-9602 · 2026-06-30T12:44:39Z

 struct DataFileMeta;
 namespace {
+Result<std::vector<std::shared_ptr<DataFileMeta>>> ReadVersion7DataFileMetaList(
+    DataInputStream* in, const std::shared_ptr<MemoryPool>& pool) {


I found that the chain table split is currently not compatible with the Java implementation. Java uses ChainSplitHeader + logicalPartition + files + bucketMap + branchMap, while C++ uses DataSplit + ChainTail. The split format must be kept fully consistent with Java.

Yes, I was working from an outdated commit. I’ll realign the implementation with the current community Java format, then submit an updated patch after further changes and verification.

lxy-9602 · 2026-07-10T01:56:37Z

+}
+
 Result<std::unique_ptr<ReaderBuilder>> AbstractSplitRead::PrepareReaderBuilder(
    const std::string& format_identifier) const {


I think there is still a semantic gap with the Java ChainSplit read path.

The PR now deserializes and preserves fileBranchMapping, and uses fileBucketPathMapping to resolve each file path. However, Java also consumes fileBranchMapping when reading each file: MergeFileSplitRead#createChainReader builds a ChainReadContext, and ChainKeyValueFileReaderFactory#getDataSchema uses chainReadContext.fileBranchMapping().get(fileMeta.fileName()) to choose the branch-specific SchemaManager before loading fileMeta.schemaId().

In the C++ path, AbstractSplitRead::CreateFieldMappingReader still loads schemas from the current table/current branch only via context_->GetTableSchema() or schema_manager_->ReadSchema(file_meta->schema_id). For a ChainSplit containing files from both snapshot and delta branches, the same schema_id may refer to different schemas across branches, or may not exist in the current branch at all. In that case C++ can read with the wrong schema or fail, while Java reads with the schema from the file’s own branch.

Could we either make ChainSplit reads branch-aware when resolving DataFileMeta::schema_id, using ChainSplitImpl::FileBranchMapping(), or explicitly fail fast for ChainSplits whose files belong to a non-current branch until branch-aware schema selection is supported?

Weixin-Xu changed the title ~~Support chain data split~~ Support to read chain data split Jun 29, 2026

zjw1111 reviewed Jun 30, 2026

View reviewed changes

zjw1111 changed the title ~~Support to read chain data split~~ feat: support to read chain data split Jun 30, 2026

lxy-9602 reviewed Jun 30, 2026

View reviewed changes

Weixin-Xu added 2 commits July 9, 2026 14:44

Support chain data split

af12574

Make raw file path factory explicit

8332390

Weixin-Xu force-pushed the support_chain_table_oss branch from 338b67a to 8332390 Compare July 9, 2026 06:57

Weixin-Xu added 2 commits July 9, 2026 15:14

Reorder ChainDataSplit tail parameter

ad8fa33

Use assign-or-raise for chain split parsing

98d0811

Weixin-Xu force-pushed the support_chain_table_oss branch from 4b4e81b to 98d0811 Compare July 9, 2026 07:31

Align chain split parser with community format

2e87f75

lxy-9602 reviewed Jul 10, 2026

View reviewed changes

Uh oh!

Conversation

Weixin-Xu commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Tests

API and Format

Documentation

Generative AI tooling

Uh oh!

CLAassistant commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zjw1111 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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Weixin-Xu commented Jun 29, 2026 •

edited

Loading

CLAassistant commented Jun 29, 2026 •

edited

Loading