Export partition to apache iceberg by arthurpassos · Pull Request #1618 · Altinity/ClickHouse

arthurpassos · 2026-04-06T13:49:44Z

Changelog category (leave one):

Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

...

Documentation entry for user-facing changes

...

CI/CD Options

Exclude tests:

Regression jobs to run:

github-actions · 2026-04-06T13:50:51Z

Workflow [PR], commit [6750617]

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5b0e833565

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-06T14:05:02Z

src/Storages/MergeTree/MergeTreeData.cpp

+        {
+            auto * object_storage = dynamic_cast<StorageObjectStorageCluster *>(dest_storage.get());
+
+            auto * iceberg_metadata = dynamic_cast<IcebergMetadata *>(object_storage->getExternalMetadata(query_context));


Guard IcebergMetadata casts with USE_AVRO

IcebergMetadata is defined only under #if USE_AVRO in IcebergMetadata.h, but this new dynamic_cast<IcebergMetadata *> is compiled unconditionally. In non-AVRO builds (USE_AVRO=0), this translation unit (and the same pattern added in StorageReplicatedMergeTree.cpp) cannot compile, so the change breaks optional-AVRO build configurations.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-06T14:05:02Z

src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp

+        const String sidecar_path = replaceFileExtensionWithAvro(
+            filename_generator.convertMetadataPathToStoragePath(path));


Use storage paths directly when reading export sidecars

The export path list is populated from filename.path_in_storage, but commit now treats each entry as a metadata path and calls convertMetadataPathToStoragePath before reading sidecars. With write_full_path_in_iceberg_metadata=1, table_dir is an URI prefix (for example s3://...) while these entries are plain storage paths (/...), so the conversion throws and EXPORT PARTITION cannot commit.

Useful? React with 👍 / 👎.

src/Storages/ExportReplicatedMergeTreePartitionManifest.h

arthurpassos · 2026-04-07T12:41:56Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6c6194814d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

src/Storages/MergeTree/MergeTreeData.cpp

src/Storages/StorageReplicatedMergeTree.cpp

… serialization logic

arthurpassos · 2026-04-10T19:23:02Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 763dec1c0a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-10T19:30:19Z

src/Storages/MergeTree/MergeTreeData.cpp

+            auto * object_storage = dynamic_cast<StorageObjectStorageCluster *>(dest_storage.get());
+
+            auto * iceberg_metadata = dynamic_cast<IcebergMetadata *>(object_storage->getExternalMetadata(query_context));


Handle non-cluster data lakes before dereferencing cast

When EXPORT PART targets a regular Iceberg table (engine backed by StorageObjectStorage, not StorageObjectStorageCluster), this dynamic_cast returns nullptr and object_storage->getExternalMetadata(...) dereferences it. That turns a user-facing validation path into a null-pointer crash instead of an error, and it is reachable because this branch runs whenever iceberg_metadata_json_ is not pre-supplied.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-10T19:30:19Z

src/Storages/StorageReplicatedMergeTree.cpp

+        auto * object_storage = dynamic_cast<StorageObjectStorageCluster *>(dest_storage.get());
+
+        auto * iceberg_metadata = dynamic_cast<IcebergMetadata *>(object_storage->getExternalMetadata(query_context));


Guard Iceberg cast in replicated partition export path

The replicated EXPORT PARTITION ... TO TABLE path unconditionally casts dest_storage to StorageObjectStorageCluster and immediately dereferences it. For local Iceberg destination tables, dest_storage is typically StorageObjectStorage, so this can null-deref before any graceful validation, causing a crash instead of returning a structured exception.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-10T19:30:19Z

src/Storages/MergeTree/ExportPartitionManifestUpdatingTask.cpp

+        std::lock_guard lock(status_changes_mutex);
+        std::swap(status_changes, local_status_changes);
+    }


Preserve unprocessed status events on handler failure

Swapping all pending status changes into a local queue means any exception during processing drops the remaining events. exportMergeTreePartitionStatusHandlingTask() catches and returns (especially on ZooKeeper/session errors), but does not requeue these keys, so some exports can miss terminal status transitions until another watch event happens (or forever if none arrives).

Useful? React with 👍 / 👎.

Fixed in 18eb4d0

chatgpt-codex-connector · 2026-04-10T19:30:19Z

src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp

+    const auto latest_schema_id = metadata->getValue<Int64>(Iceberg::f_current_schema_id);
+    if (latest_schema_id != original_schema_id)
+        throw Exception(ErrorCodes::NOT_IMPLEMENTED,
+            "Table schema changed before export could commit (expected schema {}, got {}). "
+            "Restart the export operation.",
+            original_schema_id, latest_schema_id);


Check committed transaction id before schema/spec mismatch

This code throws on schema/spec drift before running the idempotency scan. If the first commit already succeeded but the node crashed before ZooKeeper was marked completed, and the Iceberg table schema/spec changes before retry, the retry will now fail permanently instead of recognizing the existing snapshot tagged with the same transaction id.

Useful? React with 👍 / 👎.

Fixed by d9564ac

…ion and schema compliance

arthurpassos · 2026-04-13T22:30:14Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 675061716f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-13T22:43:38Z

src/Storages/ExportReplicatedMergeTreePartitionManifest.h


        manifest.lock_inside_the_task = json->getValue<bool>("lock_inside_the_task");

+        manifest.write_full_path_in_iceberg_metadata = json->getValue<bool>("write_full_path_in_iceberg_metadata");


Preserve backward compatibility for manifest parsing

Treating write_full_path_in_iceberg_metadata as mandatory breaks reading manifests that were written before this field existed. During rolling upgrades, existing ZooKeeper metadata.json entries for in-flight exports won't have this key, so getValue<bool> throws and status/polling paths that call fromJsonString cannot process those tasks. Please make this field optional with a default (false) when absent.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-13T22:43:38Z

src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.cpp

+    auto dot_pos = data_file_storage_path.rfind('.');
+    auto slash_pos = data_file_storage_path.rfind('/');
+    if (dot_pos != String::npos && (slash_pos == String::npos || dot_pos > slash_pos))
+        return data_file_storage_path.substr(0, dot_pos) + ".avro";


Avoid overwriting Avro data files with sidecars

sidecarStoragePath rewrites any extension to .avro; if the Iceberg table writes Avro data files, the data file already ends with .avro, so the sidecar path equals the data file path. writeDataFileSidecar then writes in Rewrite mode, replacing the actual data file with sidecar metadata during onFinish, which corrupts exported data for Avro-formatted Iceberg tables.

Useful? React with 👍 / 👎.

arthurpassos added 19 commits March 19, 2026 09:02

prior to llm build fix

be1fd1d

making progress

b82ce39

fix version

2687a20

export part working now

4175e33

progress before refactor

d9a27c7

checkpoint, commented out partitioning..

fdf6bf7

compatibility check seems ok

bca4aeb

simplify code

843961c

fix tst that was.. not working for some super odd reason

80753da

fix compatibility check for year vs years

fc93f76

fix

abbc77d

progress

57f3a6a

do not even recompute partition value, just use it from the source

1ee1b34

somehow fix the concurrency problem

52a812a

make it actually transactional

da5b6be

Merge branch 'antalya-26.1' into export_partition_iceberg

3664749

add test for crash during 2phase commit

a878a1e

not quite good

b5bd0eb

put writefullpath in zk and add some comments

5b0e833

try to fix fast_test

998a992

chatgpt-codex-connector bot reviewed Apr 6, 2026

View reviewed changes

arthurpassos added 5 commits April 6, 2026 11:14

again

1aa1b31

again

f21d66a

again

26827e2

partially fix path bug

6c61948

some more improvements

99ce30f

chatgpt-codex-connector bot reviewed Apr 7, 2026

View reviewed changes

src/Storages/MergeTree/MergeTreeData.cpp Show resolved Hide resolved

src/Storages/StorageReplicatedMergeTree.cpp Outdated Show resolved Hide resolved

vibe coded ffix for catalog concurrent writes

4259ec3

arthurpassos added 6 commits April 7, 2026 16:44

simplify code, calculate partition values on the fly to avoid complex…

c9dd096

… serialization logic

vibe coded tests

b180a3b

some vibe coded tests

1f73187

interesting stuff

2f5643d

is this the culprit

6a19f92

remove unused method

569803b

svb-alt added antalya antalya-26.1 labels Apr 10, 2026

arthurpassos added 3 commits April 10, 2026 08:46

one more temp fix

20aa695

fix possible deadlock

c80182c

Merge branch 'antalya-26.1' into export_partition_iceberg

763dec1

chatgpt-codex-connector bot reviewed Apr 10, 2026

View reviewed changes

arthurpassos added 4 commits April 13, 2026 11:37

interesting fix

18eb4d0

check if export has already been committed before checking for partit…

d9564ac

…ion and schema compliance

add docs around storageobjectstoragecluster cast

498ed04

lock before reading variable.. come arthur, you are better than this

6750617

chatgpt-codex-connector bot reviewed Apr 13, 2026

View reviewed changes

arthurpassos changed the title ~~[Draft] Export partition to apache iceberg~~ Export partition to apache iceberg Apr 14, 2026

		const String sidecar_path = replaceFileExtensionWithAvro(
		filename_generator.convertMetadataPathToStoragePath(path));

		auto * object_storage = dynamic_cast<StorageObjectStorageCluster *>(dest_storage.get());

		auto * iceberg_metadata = dynamic_cast<IcebergMetadata *>(object_storage->getExternalMetadata(query_context));


		manifest.lock_inside_the_task = json->getValue<bool>("lock_inside_the_task");

		manifest.write_full_path_in_iceberg_metadata = json->getValue<bool>("write_full_path_in_iceberg_metadata");

Conversation

arthurpassos commented Apr 6, 2026

Changelog category (leave one):

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Documentation entry for user-facing changes

CI/CD Options

Exclude tests:

Regression jobs to run:

Uh oh!

github-actions bot commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arthurpassos commented Apr 7, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

arthurpassos commented Apr 10, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

arthurpassos Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

arthurpassos Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

arthurpassos Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

arthurpassos Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

arthurpassos commented Apr 13, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

github-actions bot commented Apr 6, 2026 •

edited

Loading