Skip to content

Antalya 26.8: Antalya 26:6 Multiple fixes for Iceberg operations - #2462

Merged
zvonand merged 4 commits into
antalya-26.8from
feature/antalya-26.8/pr-2157
Oct 3, 2026
Merged

zvonand merged 4 commits into
antalya-26.8from
feature/antalya-26.8/pr-2157

Conversation

@zvonand

@zvonand zvonand commented Oct 1, 2026

Copy link
Copy Markdown
Member

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

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

Fixes ALTER TABLE ... ADD COLUMN, DROP COLUMN, RENAME COLUMN and MODIFY COLUMN on Iceberg tables, which could fail against a REST catalog -- DROP COLUMN of the most recently added column was rejected with Invalid last column ID, and an ALTER that the catalog had in fact applied could come back as Column already exists on retry. Bool and Decimal(P, S) columns can now be used in CREATE TABLE and ALTER TABLE ... ADD COLUMN, and a Decimal precision above the Iceberg limit of 38 is now refused up front. Dropping a column that the table's sort order or partition spec still references is now rejected, as the Iceberg specification requires (#2157 by @subkanthi).

CI/CD Options

Exclude tests:

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

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • S3 Export (2h)
  • Swarms (30m)
  • Tiered Storage (2h)

Cherry-picked from #2157.


continuation of work from #1841

@zvonand zvonand added releasy Created/managed by RelEasy antalya-26.8 Session label (releasy session config) forwardport This is a frontport of code that existed in previous Antalya versions ai-resolved Port conflict auto-resolved by Claude labels Oct 1, 2026
@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Workflow [PR], commit [cca2ac5]

@svb-alt svb-alt added the antalya label Oct 1, 2026
@zvonand zvonand mentioned this pull request Oct 1, 2026
36 tasks done
…ceberg

Antalya 26:6 Multiple fixes for Iceberg operations
@zvonand

zvonand commented Oct 2, 2026

Copy link
Copy Markdown
Member Author

@blau-ai

@blau-ai

blau-ai commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

CI triage for #2462 (feature/antalya-26.8/pr-2157 → antalya-26.8)

Verdict: 5 failing checks → 3 PR-caused test failures (2 root causes), 2 not PR-caused.

The two PR-caused issues both come from the #2157 cherry-pick: the source change landed but a matching base-branch behaviour/test wasn't carried along. Both are small, concrete fixes. Note all three integration-test suites are marked do_not_block_pipeline_on_failure: true, so they don't gate the merge — but they are real regressions worth fixing.


🔴 PR-caused #1 — getIcebergType now rejects wide decimals (Decimal256)

Check: Integration tests (arm_binary, distributed plan, 1/4) — Failures: 2/2117
Tests: test_storage_iceberg_with_spark/test_writes.py::test_writes_decimal_wide_minmax_pruning[s3] and [local] (both retry_failed — deterministic, not flaky)

Code: 36. DB::Exception: Iceberg decimal type supports precision up to 38, got 76. (BAD_ARGUMENTS)
  at src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.cpp:636  DB::Iceberg::getIcebergType(...)
  (query: CREATE TABLE ... (d128 Decimal(38, 10), d256 Decimal(76, 20), control Int64) ENGINE=IcebergS3/Local ...)

Cause: The base branch antalya-26.8 supports Decimal256 in Iceberg:

// fbe84302a88:src/.../Iceberg/Utils.cpp
case TypeIndex::Decimal256:
    return {fmt::format("decimal({}, {})", getDecimalPrecision(*type), getDecimalScale(*type)), true};

The #2157 cherry-pick replaced that with the upstream precision > 38 → throw block (added as + in this PR's diff). That clobbers Altinity's wide-decimal support, so the pre-existing, untouched test_writes_decimal_wide_minmax_pruning test — whose whole point is Decimal128/Decimal256 min/max pruning — now fails at CREATE TABLE.

Suggested fix — restore the base-branch handling in Utils.cpp (getIcebergType):

        case TypeIndex::Decimal32:
        case TypeIndex::Decimal64:
        case TypeIndex::Decimal128:
        case TypeIndex::Decimal256:
            return {fmt::format("decimal({}, {})", getDecimalPrecision(*type), getDecimalScale(*type)), true};

⚠️ Judgement call for you: if #2157 intended to reject >38-precision decimals (Iceberg spec caps decimal precision at 38), then the correct fix is instead to remove/adjust test_writes_decimal_wide_minmax_pruning and drop the wide-decimal feature deliberately. The conflict resolution currently does neither cleanly — it took upstream's stricter check but kept the base test that requires the looser behaviour. You know #2157's intent; tell me which way and I'll prepare it.


🔴 PR-caused #2 — RestCatalog conflict log renamed, pre-existing test still greps the old text

Check: Integration tests (arm_binary, distributed plan, 2/4) — Failures: 1/1534
Test: test_database_iceberg/test.py::test_catalog_commit_conflict_reaches_caller_at_once (retry_failed)

AssertionError: no writer was refused, so nothing about conflict handling was exercised
  at test_database_iceberg/test.py:2846   assert conflicts

Cause: This PR changed the conflict log line in RestCatalog::updateMetadata:

- LOG_DEBUG(log, "updateMetadata conflict for {}/{}: {}", namespace_name, table_name, ...);
+ LOG_WARNING(log, "Iceberg REST updateMetadata for {}.{} got retryable HTTP {}: {}", ...);   // RestCatalog.cpp:2306

The pre-existing, untouched test counts conflicts by the old message substring:

# test.py:2838
countIf(logger_name LIKE 'RestCatalog%' AND message LIKE '%updateMetadata conflict%'),

Conflicts do still happen (writers still get HTTP 409), but the string updateMetadata conflict no longer exists in any log line, so conflicts counts 0 and the assertion trips. This is a stale test, not a behaviour regression.

Suggested fix — update the test predicate to match the new message:

# test.py:2838
                countIf(logger_name LIKE 'RestCatalog%' AND message LIKE '%updateMetadata%got retryable HTTP 409%'),

(The surrounding WHERE ... message LIKE '%409%' already scopes to 409s, so this keeps the "one catalog request per conflict" check intact.)


🟢 Not PR-caused #1 — test_s3_cluster graceful shutdown

Check: Integration tests (arm_binary, distributed plan, 3/4) — Failures: 1/1959
Test: test_s3_cluster/test.py::test_graceful_shutdown — assert errors == 0 → assert 1 == 0

Unrelated to Iceberg; test_s3_cluster/ is not touched by this PR and doesn't exercise any changed code path. This is the familiar graceful-shutdown error-count flake. Safe to re-run; no PR action needed. (No existing flaky issue found for it — worth opening one if it recurs.)

🟢 Not PR-caused #2 — Grype image scans

Checks: GrypeScanKeeper (4 high/critical) and GrypeScanServer (-alpine) (1 high/critical).

Container-image CVE scans of the keeper and alpine-server images; they flag base-OS/package vulnerabilities and are independent of this PR's C++/Python changes. Pre-existing/infra — not caused by #2462. These typically need a base-image bump handled separately from feature PRs.


Health check

Everything that actually exercises this PR's code is green except the two issues above — Fast test (11197 passed), all Builds, Stateless and the other ~5600 integration tests pass. Both PR-caused failures are narrow mismatches introduced by the cherry-pick conflict resolution (a clobbered base-branch decimal path + a renamed log string a stale test depends on), not logic bugs in the feature itself.

Delivery: per policy I haven't pushed anything. Want me to (a) commit both fixes directly to feature/antalya-26.8/pr-2157, or (b) open a separate blau/* PR against it? And for decimal fix #1, confirm whether wide decimals should stay supported (restore base behaviour) or be rejected (drop the test) so I apply the right one.

Evidence: praktika result_pr.json for 83c2dc7; diff fbe84302a88..HEAD. I can't build/run CH here, so correctness is validated by re-running CI after the fix.

#2157 refuses an Iceberg `decimal` with precision above 38, as the Iceberg
spec requires, so the `Decimal(76, 20)` column fails at `CREATE TABLE`.
Same change as efe732b on `antalya-26.6`.

CI report: #2462 (comment)
Related: #2462
…_reaches_caller_at_once`

#2157 replaced the `updateMetadata conflict for ...` log line in
`RestCatalog::updateMetadata` with `Iceberg REST updateMetadata for ... got
retryable HTTP 409: ...`, so the test counted zero conflicts.

CI report: #2462 (comment)
Related: #2462
@zvonand
zvonand force-pushed the feature/antalya-26.8/pr-2157 branch from 83c2dc7 to cca2ac5 Compare October 2, 2026 18:31
@zvonand
zvonand merged commit a6b6f62 into antalya-26.8 Oct 3, 2026
317 of 321 checks passed
@zvonand zvonand added verified Approved for release port-antalya PRs to be ported to all new Antalya releases labels Oct 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-resolved Port conflict auto-resolved by Claude antalya 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 releasy Created/managed by RelEasy verified Approved for release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants