Skip to content

Antalya-26.8: Iceberg: support external paths in tables - #2551

Merged
zvonand merged 5 commits into
antalya-26.8from
backport/antalya-26.8/90740
Oct 14, 2026
Merged

zvonand merged 5 commits into
antalya-26.8from
backport/antalya-26.8/90740

Conversation

@zvonand

@zvonand zvonand commented Oct 12, 2026

Copy link
Copy Markdown
Member

Iceberg: support external paths in tables

Adaptations to antalya-26.8:

  • The drop flow here is prepareForDrop(query_context) + IDataLakeMetadata::drop(bool delete_data) (from the backport of PR 98670), not upstream's drop(ContextPtr). checkCanBeDropped keeps the PR's checkTableCanBeDropped hook, reads data_lake_delete_data_on_drop from the query context (iceberg_delete_data_on_drop is only an alias here), and drop(bool) enumerates the external files with the global context. StorageIcebergConfiguration forwards checkCanBeDropped like drop.
  • DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_ABSOLUTE_PATH keeps upstream's number 12; versions 10 (CDC reading) and 11 (deletion vectors) do not exist here, so the branch jumps from 9 to 12. The protocol spec text drops the mention of version 11.
  • Settings.cpp here has no inline settings-changes history, so the entry for object_storage_propagate_credentials_to_other_storages is recorded in SettingsChangesHistory.cpp (26.6.2.20001.altinityantalya section, as the previous backport did).
  • ObjectStorageFactory::create takes a single skip_access_check here; AzureObjectStorage takes auth_method; S3::URI and getManifestFile keep their extra branch-specific parameters.
  • Deletion vectors are loaded on the coordinator in IcebergIterator here (no deletion_vector in IcebergObjectSerializableInfo, no readIcebergDeletionVector), so the storage resolution for the Puffin file is applied to loadDeletionVector and the deletion-vector parts of the protocol serialization and of the cluster compatibility check are dropped. The serde unit test uses protocol version 9 as the "old" version.
  • collectReachableFiles has no catalog parameters here; remove_orphan_files keeps the version-based TOCTOU check (no metadata_path in ReachableFilesResult).
  • totalRows here never uses the snapshot summary; only the isPathReadable check is added.
  • AlterDropPartitionExecutor, test_cross_bucket_data_files.py, 05043_object_storage_path_virtual_column_empty_namespace and the documentation block in TableFunctionObjectStorage.cpp do not exist here; the PR's changes to them are dropped. DatabaseDataLake::buildTableEngineArgs does not exist; the Local storage exemption is applied to the equivalent credentials check.
  • Upstream-only context (masked URI helpers in URI.cpp, foreign-namespace helpers in IcebergPath.cpp, row-lineage code, makeQueryConditionCacheKey, read_settings in the position-delete transform) is not pulled in.

Changelog category (leave one):

  • Improvement

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

Support Iceberg tables that have files outside table location or on different storage. The _path virtual column for Iceberg data files now reports the path as written in the Iceberg manifest (for example s3://bucket/table/data/p.parquet) instead of the previous namespace/key form, and system.iceberg_files.file_path reports it the same way.

CI/CD Options

Exclude tests:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Unit tests
  • Performance tests
  • Aarch64 tests
  • All with ASAN
  • All with TSAN
  • All with MSAN
  • All with UBSAN
  • All with Coverage
  • All Regression
  • Disable CI Cache

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • CAS (content-addressed storage; Antalya only)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • OAuth (5m)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • S3 Export (2h)
  • Swarms (30m)
  • Tiered Storage (2h)

Iceberg: support external paths in tables

Adaptations to antalya-26.8:
- The drop flow here is `prepareForDrop(query_context)` + `IDataLakeMetadata::drop(bool delete_data)`
  (from the backport of PR 98670), not upstream's `drop(ContextPtr)`. `checkCanBeDropped` keeps the
  PR's `checkTableCanBeDropped` hook, reads `data_lake_delete_data_on_drop` from the query context
  (`iceberg_delete_data_on_drop` is only an alias here), and `drop(bool)` enumerates the external
  files with the global context. `StorageIcebergConfiguration` forwards `checkCanBeDropped` like `drop`.
- `DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION_WITH_ICEBERG_ABSOLUTE_PATH` keeps upstream's number 12;
  versions 10 (CDC reading) and 11 (deletion vectors) do not exist here, so the branch jumps from 9 to 12.
  The protocol spec text drops the mention of version 11.
- `Settings.cpp` here has no inline settings-changes history, so the entry for
  `object_storage_propagate_credentials_to_other_storages` is recorded in `SettingsChangesHistory.cpp`
  (`26.6.2.20001.altinityantalya` section, as the previous backport did).
- `ObjectStorageFactory::create` takes a single `skip_access_check` here; `AzureObjectStorage` takes
  `auth_method`; `S3::URI` and `getManifestFile` keep their extra branch-specific parameters.
- Deletion vectors are loaded on the coordinator in `IcebergIterator` here (no `deletion_vector` in
  `IcebergObjectSerializableInfo`, no `readIcebergDeletionVector`), so the storage resolution for
  the Puffin file is applied to `loadDeletionVector` and the deletion-vector parts of the protocol
  serialization and of the cluster compatibility check are dropped. The serde unit test uses
  protocol version 9 as the "old" version.
- `collectReachableFiles` has no catalog parameters here; `remove_orphan_files` keeps the
  version-based TOCTOU check (no `metadata_path` in `ReachableFilesResult`).
- `totalRows` here never uses the snapshot summary; only the `isPathReadable` check is added.
- `AlterDropPartitionExecutor`, `test_cross_bucket_data_files.py`,
  `05043_object_storage_path_virtual_column_empty_namespace` and the documentation block in
  `TableFunctionObjectStorage.cpp` do not exist here; the PR's changes to them are dropped.
  `DatabaseDataLake::buildTableEngineArgs` does not exist; the `Local` storage exemption is applied
  to the equivalent credentials check.
- Upstream-only context (masked URI helpers in `URI.cpp`, foreign-namespace helpers in
  `IcebergPath.cpp`, row-lineage code, `makeQueryConditionCacheKey`, `read_settings` in the
  position-delete transform) is not pulled in.
@zvonand zvonand added port-antalya PRs to be ported to all new Antalya releases forwardport This is a frontport of code that existed in previous Antalya versions antalya-26.8 Session label (releasy session config) labels Oct 12, 2026
@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Workflow [PR], commit [3894ea0]

The cherry-pick of ClickHouse#90740 took upstream's
`DBMS_CLUSTER_PROCESSING_PROTOCOL_VERSION` slot 12 for sending the Iceberg delete-file paths as
absolute URIs in the cluster read-task payload. Antalya versions its own wire changes with
`DBMS_ANTALYA_PROTOCOL_VERSION`, negotiated per connection in the `Hello`
(#2487), so the upstream bump is undone and the change is
gated on Antalya protocol version 2 instead. The negotiated version of the worker is passed from
`Connection` through `ClusterFunctionReadTaskResponse::serialize` to
`IcebergObjectSerializableInfo::serializeForClusterFunctionProtocol`.
Compare the literal legacy wire key with the resolved delete-file key. Require Antalya protocol version 2 when they differ, including copied tables, instead of resolving the stripped key again. Remove the exception-catching fallback.

Correct the cluster protocol specification to version 9 and cover copied-table paths and Antalya versions 0, 1, and 2. Built `clickhouse` and `unit_tests_dbms`; all 31 focused tests passed.

Related: #2551
Port the approach of #2268 to the
`test_external_paths_cross_bucket.py` version of the test: instead of corrupting
`record_count` and dropping `total-records`, write a live merge-on-read position
delete file so that `count()` has to scan the data files, and relocate only the
data file without deletes to the shared key in the other bucket.

Related: #2268
@zvonand zvonand mentioned this pull request Oct 13, 2026
51 tasks done
@zvonand

zvonand commented Oct 14, 2026

Copy link
Copy Markdown
Member Author

@blau-ai

@blau-ai

This comment was marked as outdated.

@zvonand

zvonand commented Oct 14, 2026

Copy link
Copy Markdown
Member Author

@blau-ai

@blau-ai

blau-ai commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator

CI triage

Verdict: 6 failing checks, 0 caused by this PR. All are pre-existing / infra / flaky on antalya-26.8 and reproduce on unrelated PRs. Every check that actually exercises this PR's code is green: Fast test (11205 passed), all Builds (amd debug/asan+ubsan/binary/release, arm release, wasm), Stateless (13260 + 12973 + 951 + 893 passed), Integration targeted (no failures), AST fuzzer — including this PR's own new integration test test_storage_iceberg_with_spark/test_external_paths_access.py.

No code change is needed on this PR. Details per failure below.

Not PR-related (infra / build-env)

Grype Scan — keeper (4 High/Crit), server -alpine (1 High)
Trivy/Grype CVE scans of the Alpine base-image OS packages, not the C++ diff. Findings are Alpine 3.21/3.24 distro CVEs (CVE-2026-84782/84784/85091/72897). The PR touches no Dockerfile, base image, or vendored dep. Same two checks fail with identical counts on unrelated open PRs #2548 and #2545.

Parser memory check — "Failures: 1/2"
No parser/grammar test ran. The failing sub-step is "Resolve master binary": the WASM-parser memory comparison needs a baseline clickhouse-examples artifact from a master/base commit and none was published (No master binary found in 100 tracked commits). The PR doesn't touch src/Parsers. Same "Failures: 1/2" on #2548 and #2545 → branch-wide infra.

Not PR-related (regression-suite env / stale assertions / flake)

Regression → oauth — test-expectation bug, server behaves correctly.
Failing: access control/missing_default_profile_does_not_grant_unrestricted_access, groups/group_name_matches_clickhouse_role (the 298 "retries failed" are just these two retried). Error: Expected HTTP status code 500, but got 403 … Code: 497 … Not enough privileges … (ACCESS_DENIED). The server correctly denies with 403; the clickhouse-regression helper hardcodes status_code=500. ClickHouse maps ACCESS_DENIED→403, so this is a stale assertion in the test repo, independent of any server change here.

Regression → s3_azure_1 — environment, server won't start.
Whole /s3/azure tree fails at health-check with URL "http://azurite:10000" is not allowed … <remote_url_allow_hosts> (UNACCEPTABLE_URL) plus a tlsv1 alert unknown ca probe. A code regression wouldn't be so selective: s3_azure_2 and every other storage variant on this same run passed.

Regression → swarms — single flaky negative scenario.
1518/1535 scenarios passed; only cluster discovery/check_cluster_discovery_with_wrong_cluster_name failed with Code: 279 … Cannot connect to any replica (ALL_CONNECTION_TRIES_FAILED) (expected exitcode 23, got 33). The 1518 passing s3Cluster scenarios rule out the PR's protocol/cluster changes. Same suite is red on unrelated merged PRs #2547 and #2558.

Regression → tiered_storage_cas — build/feature gap, server won't start.
MetadataStorageFactory: unknown metadata storage type: cas (UNKNOWN_ELEMENT_IN_CONFIG) → config reload fails, server never comes up. The build simply lacks the cas metadata-storage type; all other tiered_storage variants (local/minio/s3amazon/s3gcs) pass on this run, and the suite is red on unrelated PR #2556.

Why the PR's non-Iceberg changes don't explain any of these

The diff does touch Access/Connection/protocol, but narrowly:

  • src/Access/ContextAccess.cpp: normalizes the URI/path only in isGrantedWithFilter for filtered grants (GRANT READ ON S3('…') / FILE source) — the Iceberg external-path access check. It doesn't touch ordinary privilege checks (CREATE TEMPORARY TABLE, SELECT ON system.users) that the oauth failures hit.
  • src/Client/Connection.cpp + src/Core/AntalyaProtocol.h: bump the Antalya protocol version and thread it into *Cluster read-task serialization (Iceberg delete-file absolute paths). If broken it would break s3Cluster broadly — but 1518 swarms s3Cluster scenarios passed.

Suggested next step

Nothing to fix in this PR. The red checks are safe to re-run / ignore for merge purposes. The oauth (stale 500-vs-403 assertion), s3_azure (remote_url_allow_hosts/cert), tiered_storage cas, and Grype/parser-baseline issues are branch-wide infra items worth a separate ticket, not a change to #2551.

@zvonand
zvonand merged commit d1da97e into antalya-26.8 Oct 14, 2026
298 of 313 checks passed
@zvonand zvonand added the verified Approved for release label Oct 14, 2026
zvonand added a commit that referenced this pull request Oct 15, 2026
`AlterDropPartitionExecutor` (#2533) called
`getManifestFileEntriesHandle` and `generateManifestList` without the `ExternalStorageCache`
argument added by #2551. Pass the cache of
`IcebergMetadata` to the executor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

antalya-26.8 Session label (releasy session config) forwardport This is a frontport of code that existed in previous Antalya versions port-antalya PRs to be ported to all new Antalya releases verified Approved for release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants