S3Tables create DDL fix

[scanhex12]

Changelog category (leave one):

• Improvement

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

Allow to create own s3 tables

---

Workflow [PR]
Sync PR [sync-upstream/pr/115652]

Version info

• Merged into: 26.8.1.1834 (included in 26.8 and later)

[clickhouse-gh]

Workflow [PR], commit [dcf6898]

Summary: ✅

• Performance Comparison: Performance dashboard

---

AI Review

Summary

This PR tries to make CREATE TABLE work for Iceberg tables backed by the S3TablesCatalog by teaching the shared REST/Iceberg code that the catalog owns table locations. The current patch still leaves the namespace-create request shape, the table-create request shape, and the IF NOT EXISTS conflict path inconsistent with the S3 Tables contract, so the advertised create flow is still not reliable.

Missing context / blind spots

• ⚠️ Most CI shards were still pending while I reviewed; only Fast test, Style check, and Build (arm_tidy) had finished.

Findings

• ❌ Blockers
  • [src/Databases/DataLake/RestCatalog.cpp:1753] S3TablesCatalog now suppresses location, but it still inherits the shared stage-create = false payload here. That means CREATE TABLE against the real S3 Tables REST endpoint can still be rejected even after this patch. Suggested fix: move S3 Tables creation onto an override or capability branch that omits S3-unsupported request fields, not just location.
  • [src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp:858, src/Databases/DataLake/RestCatalog.cpp:1712-1714] createInitial still calls the generic REST namespace-create helper, and that helper always POSTs properties.location. For S3 Tables the namespace request shape is different, so the first table created in a fresh namespace can still fail before table creation starts. Suggested fix: add an S3-specific createNamespaceIfNotExists implementation or a capability that suppresses namespace location properties when the catalog owns placement.
  • [src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp:804-809, src/Databases/DataLake/RestCatalog.cpp:1761-1767] The new existence probe does not make CREATE TABLE IF NOT EXISTS race-safe. Two concurrent creators can both observe "missing", one POST succeeds, and the loser still rethrows the catalog conflict as DATALAKE_DATABASE_ERROR instead of turning it into the normal no-op. Suggested fix: treat catalog create conflicts like the non-managed If-None-Match path and swallow or re-check them when if_not_exists is set.

Tests

• ⚠️ Add a focused regression test for S3 Tables CREATE TABLE that exercises a fresh namespace and verifies the emitted REST payload omits the S3-unsupported fields on both namespace create and table create.
• ⚠️ Add a focused conflict test for CREATE TABLE IF NOT EXISTS on catalog-managed locations so a concurrent create conflict is swallowed instead of surfacing as DATALAKE_DATABASE_ERROR.

Final Verdict

• Status: ❌ Block
• Minimum required actions: fix the S3 Tables request shapes for both namespace and table creation, make IF NOT EXISTS conflict-tolerant on the catalog-managed path, and add focused coverage for those cases.

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	87.00%	87.00%	+0.00%
Functions	91.90%	91.90%	+0.00%
Branches	79.30%	79.30%	+0.00%

Changed lines: Changed C/C++ lines covered: 82/95 (86.32%) · Uncovered code

Full report · Diff report

[clickhouse-gh]

S3TablesCatalog still goes through this shared request builder, and a few lines below we unconditionally add stage-create = false. AWS documents the S3 Tables REST endpoint as rejecting the stage-create option entirely, so omitting location here is not enough: CREATE TABLE can still fail with 400 Bad Request against the real service.

I think this needs an S3TablesCatalog::createTable override (or a managesTableLocation() branch) that suppresses stage-create altogether.

[clickhouse-gh]

This still calls the base REST namespace-create helper, which always POSTs properties.location. For S3 Tables that is not a supported namespace property, so the first CREATE TABLE in a fresh namespace is still likely to fail even after the table-location changes.

We probably need an S3-specific createNamespaceIfNotExists implementation, or a catalog capability that suppresses the location property for namespace creation as well.

[clickhouse-gh]

This existence probe does not make IF NOT EXISTS race-safe. Two concurrent creators can both observe "missing" here, one createTable succeeds, and the loser then bubbles the catalog conflict back as DATALAKE_DATABASE_ERROR instead of treating it as "already exists".

The non-managed path closes the same gap with If-None-Match on the metadata write. We need equivalent conflict handling here, e.g. surface HTTP_CONFLICT distinctly from createTable and swallow it when if_not_exists is set, or re-check existence after the conflict.

[clickhouse-gh]

Build profile diff (arm_release)

Comparing dcf6898b5 with master e0bb74066 (stripped binary size, per-symbol sizes and ThinLTO time; compile times per translation unit against the most recent warmup build that recompiled it).

✅ No significant changes.

Binary sizes

Binary	Master	PR	Δ
programs/clickhouse-stripped	704.49 MiB	701.43 MiB	-3.05 MiB (-0.43%)

Only the stripped binary is compared: the official master build keeps debug symbols while PR builds strip them, so the other binaries differ by construction.

Compile time of recompiled translation units

112 translation units recompiled, 1041 s compile time in total, 112 of them have a recent master baseline.

Job report