iframe-proxy

scanhex12 · 2026-05-29T11:49:46Z

Changelog category (leave one):

Improvement

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

catalogs with schema evolutions + concurrent checks

Version info

Merged into: 26.6.1.431

clickhouse-gh · 2026-05-29T11:50:22Z

Workflow [PR], commit [710c69c]

Summary: ❌

Performance Comparison: Performance dashboard

job_name	test_name	status	info
Stress test (arm_release)		FAIL
	Assertion 'px != 0' failed (STID: 2659-2662)	FAIL	cidb
BuzzHouse (amd_debug)		FAIL
	Logical error: 'Inconsistent AST formatting: the original AST: (STID: 1941-1bfa)	FAIL	cidb

AI Review

Summary

This PR adds catalog-backed Iceberg schema evolution for REST and Glue catalogs and adds concurrent schema-change integration coverage. The retry path is closer now because MetadataGenerator rejects stale ADD COLUMN, MODIFY COLUMN, DROP COLUMN, and RENAME COLUMN commands after reloading newer metadata, but one catalog error-handling issue remains and the added concurrency test still does not prove the invalid-after-retry case.

PR Metadata

Changelog category: Improvement is reasonable for this change.
Changelog entry: required and currently too vague: catalogs with schema evolutions + concurrent checks does not tell users which feature changed or what behavior is now supported.
Suggested replacement: Support ALTER TABLE schema evolution for Iceberg tables backed by REST and Glue catalogs, including optimistic retries for concurrent schema changes.

Findings

⚠️ Majors

[src/Databases/DataLake/RestCatalog.cpp:1337] [dismissed by author -- https://github.com/catalogs with schema evolutions + concurrent checks #106102#discussion_r3324146860] updateSchema documents false as a recoverable optimistic-concurrency conflict, but the current code still catches every HTTPException and returns false unconditionally. Permanent catalog failures such as malformed updates, auth failures, missing tables, or server errors will be retried and finally reported as Too many unsuccessed retries to alter iceberg table, hiding the real catalog response. Return false only for the conflict/precondition status used by assert-current-schema-id; rethrow or wrap all other statuses with the catalog error.

Tests

⚠️ [tests/integration/test_database_iceberg/test.py:1043] The new concurrent test proves that distinct ADD COLUMN retries are merged, but not that a command that became invalid after retry is rejected. Add a focused case where two workers both run ALTER TABLE ... ADD COLUMN z Nullable(String), assert exactly one succeeds, the other reports the normal already-exists error, and the final schema contains a single z. Mirroring this for Glue would also cover the non-transactional catalog path.

Final Verdict

Status: ⚠️ Request changes

Minimum required actions: narrow REST catalog retry handling to real schema conflicts, and add the focused concurrent conflict regression test.

clickhouse-gh · 2026-05-29T12:41:20Z

+
+    def add_column(idx):
+        node.query(
+            f"ALTER TABLE {table_ref} ADD COLUMN col_{idx} Nullable(String);",


This concurrent test exercises the happy path where every retry adds a different column, but it does not cover the invariant this PR had to enforce on retry: after reloading newer catalog metadata, a command that became invalid must fail instead of committing another schema.

Please add a focused concurrent conflict case, for example two workers both running ALTER TABLE ... ADD COLUMN z Nullable(String) and asserting that exactly one succeeds while the other gets the normal "already exists" error and the final schema contains a single z. That would prove the new MetadataGenerator validation is actually wired into the retry path.

clickhouse-gh · 2026-05-29T15:55:59Z

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	84.40%	84.40%	+0.00%
Functions	91.40%	91.40%	+0.00%
Branches	76.90%	77.00%	+0.10%

Changed lines: Changed C/C++ lines covered by tests: 63/167 (37.72%) | Lost baseline coverage (was covered on master, now uncovered in this PR): 2 line(s) · Uncovered code

Full report · Diff report

PedroTadim · 2026-06-05T18:42:17Z

…rg::alter PR ClickHouse#106102 (merged 2026-06-05 17:43 UTC) refactored Iceberg::alter to declare last_version and compression_method outside an if/else that assigns them in both branches. Clang-tidy cppcoreguidelines-init-variables flags the bare declarations and Build (arm_tidy) is built with -warnings-as-errors, so the build fails for every PR whose CI ran on master after this commit. CIDB shows 30+ unrelated PRs hitting this in the last 2 days (e.g. ClickHouse#89360, ClickHouse#103540, ClickHouse#106120, ClickHouse#106386, ClickHouse#106404, ClickHouse#106522, ClickHouse#105102, ClickHouse#106590). Initialize both variables to safe defaults at declaration. They are unconditionally overwritten in both if and else branches before use, so behavior is unchanged. The new defaults are only relevant if a future code path skips both assignments, in which case last_version=0 and compression_method=CompressionMethod::None are sane no-op values (the same defaults the old structured-binding form would produce through default-construction of the destructured aggregate). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit d72cac5)

…rg::alter PR #106102 (merged 2026-06-05 17:43 UTC) refactored Iceberg::alter to declare last_version and compression_method outside an if/else that assigns them in both branches. Clang-tidy cppcoreguidelines-init-variables flags the bare declarations and Build (arm_tidy) is built with -warnings-as-errors, so the build fails for every PR whose CI ran on master after this commit. CIDB shows 30+ unrelated PRs hitting this in the last 2 days (e.g. #89360, #103540, #106120, #106386, #106404, #106522, #105102, #106590). Initialize both variables to safe defaults at declaration. They are unconditionally overwritten in both if and else branches before use, so behavior is unchanged. The new defaults are only relevant if a future code path skips both assignments, in which case last_version=0 and compression_method=CompressionMethod::None are sane no-op values (the same defaults the old structured-binding form would produce through default-construction of the destructured aggregate). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit d72cac5)

…rg::alter PR ClickHouse#106102 (merged 2026-06-05 17:43 UTC) refactored Iceberg::alter to declare last_version and compression_method outside an if/else that assigns them in both branches. Clang-tidy cppcoreguidelines-init-variables flags the bare declarations and Build (arm_tidy) is built with -warnings-as-errors, so the build fails for every PR whose CI ran on master after this commit. CIDB shows 30+ unrelated PRs hitting this in the last 2 days (e.g. ClickHouse#89360, ClickHouse#103540, ClickHouse#106120, ClickHouse#106386, ClickHouse#106404, ClickHouse#106522, ClickHouse#105102, ClickHouse#106590). Initialize both variables to safe defaults at declaration. They are unconditionally overwritten in both if and else branches before use, so behavior is unchanged. The new defaults are only relevant if a future code path skips both assignments, in which case last_version=0 and compression_method=CompressionMethod::None are sane no-op values (the same defaults the old structured-binding form would produce through default-construction of the destructured aggregate). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit d72cac5)

…rg::alter PR #106102 (merged 2026-06-05 17:43 UTC) refactored Iceberg::alter to declare last_version and compression_method outside an if/else that assigns them in both branches. Clang-tidy cppcoreguidelines-init-variables flags the bare declarations and Build (arm_tidy) is built with -warnings-as-errors, so the build fails for every PR whose CI ran on master after this commit. CIDB shows 30+ unrelated PRs hitting this in the last 2 days (e.g. #89360, #103540, #106120, #106386, #106404, #106522, #105102, #106590). Initialize both variables to safe defaults at declaration. They are unconditionally overwritten in both if and else branches before use, so behavior is unchanged. The new defaults are only relevant if a future code path skips both assignments, in which case last_version=0 and compression_method=CompressionMethod::None are sane no-op values (the same defaults the old structured-binding form would produce through default-construction of the destructured aggregate). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit d72cac5)

…rg::alter PR ClickHouse#106102 (merged 2026-06-05 17:43 UTC) refactored Iceberg::alter to declare last_version and compression_method outside an if/else that assigns them in both branches. Clang-tidy cppcoreguidelines-init-variables flags the bare declarations and Build (arm_tidy) is built with -warnings-as-errors, so the build fails for every PR whose CI ran on master after this commit. CIDB shows 30+ unrelated PRs hitting this in the last 2 days (e.g. ClickHouse#89360, ClickHouse#103540, ClickHouse#106120, ClickHouse#106386, ClickHouse#106404, ClickHouse#106522, ClickHouse#105102, ClickHouse#106590). Initialize both variables to safe defaults at declaration. They are unconditionally overwritten in both if and else branches before use, so behavior is unchanged. The new defaults are only relevant if a future code path skips both assignments, in which case last_version=0 and compression_method=CompressionMethod::None are sane no-op values (the same defaults the old structured-binding form would produce through default-construction of the destructured aggregate). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit d72cac5)

…rg::alter PR #106102 (merged 2026-06-05 17:43 UTC) refactored Iceberg::alter to declare last_version and compression_method outside an if/else that assigns them in both branches. Clang-tidy cppcoreguidelines-init-variables flags the bare declarations and Build (arm_tidy) is built with -warnings-as-errors, so the build fails for every PR whose CI ran on master after this commit. CIDB shows 30+ unrelated PRs hitting this in the last 2 days (e.g. #89360, #103540, #106120, #106386, #106404, #106522, #105102, #106590). Initialize both variables to safe defaults at declaration. They are unconditionally overwritten in both if and else branches before use, so behavior is unchanged. The new defaults are only relevant if a future code path skips both assignments, in which case last_version=0 and compression_method=CompressionMethod::None are sane no-op values (the same defaults the old structured-binding form would produce through default-construction of the destructured aggregate). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit d72cac5)

start

dedbc2b

clickhouse-gh Bot added the pr-improvement Pull request with some product improvements label May 29, 2026

clickhouse-gh Bot reviewed May 29, 2026

View reviewed changes

Comment thread src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.cpp

clickhouse-gh Bot reviewed May 29, 2026

View reviewed changes

Comment thread src/Databases/DataLake/RestCatalog.cpp Outdated

scanhex12 added 2 commits May 29, 2026 12:30

add more validation

893f14e

add more logs

710c69c

clickhouse-gh Bot reviewed May 29, 2026

View reviewed changes

scanhex12 mentioned this pull request Jun 3, 2026

Fix mutation issues #106111

Merged

scanhex12 self-assigned this Jun 5, 2026

scanhex12 added this pull request to the merge queue Jun 5, 2026

Merged via the queue into ClickHouse:master with commit 9c9b2b9 Jun 5, 2026
163 of 166 checks passed

scanhex12 deleted the fix_alter_catalog branch June 5, 2026 17:59

robot-ch-test-poll4 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jun 5, 2026

This was referenced Jun 5, 2026

Do not lift arrayJoin above SortingStep (issue #82279) #104558

Merged

Fix projection rewrite collapsing groups when WHERE and GROUP BY share a column #106590

Merged

Fix arm_tidy: initialize variables in Iceberg::alter (caused by #106102) #106615

Closed

mstetsyuk mentioned this pull request Jun 5, 2026

Reapply "Add UntrackedMemory asynchronous metric" #106386

Open

groeneai mentioned this pull request Jun 6, 2026

Treat float NaN keys as NULL on JOIN ON #106540

Open

alexey-milovidov mentioned this pull request Jun 7, 2026

Support specifying or auto-assigning Parquet field_ids for output columns #101783

Open

1 task

groeneai mentioned this pull request Jun 11, 2026

Fix LimitTransform WITH TIES assertion on empty virtual-row chunk #105102

Open

1 task

Sunbelt Computer Software

PL/B Language Development and Support

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

catalogs with schema evolutions + concurrent checks#106102

catalogs with schema evolutions + concurrent checks#106102
scanhex12 merged 3 commits into
ClickHouse:masterfrom
scanhex12:fix_alter_catalog

scanhex12 commented May 29, 2026 •

edited by robot-clickhouse

Loading

Uh oh!

clickhouse-gh Bot commented May 29, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

clickhouse-gh Bot May 29, 2026

Uh oh!

clickhouse-gh Bot commented May 29, 2026

Uh oh!

Uh oh!

PedroTadim commented Jun 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Conversation

scanhex12 commented May 29, 2026 • edited by robot-clickhouse Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog category (leave one):

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

Version info

Uh oh!

clickhouse-gh Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AI Review

Summary

PR Metadata

Findings

Tests

Final Verdict

Uh oh!

Uh oh!

Uh oh!

clickhouse-gh Bot May 29, 2026

Choose a reason for hiding this comment

Uh oh!

clickhouse-gh Bot commented May 29, 2026

LLVM Coverage Report

Uh oh!

Uh oh!

PedroTadim commented Jun 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

scanhex12 commented May 29, 2026 •

edited by robot-clickhouse

Loading

clickhouse-gh Bot commented May 29, 2026 •

edited

Loading