Conversation
…-alter-table-modify-column-in-iceberg-tables
|
|
…-column-in-iceberg-tables' of https://github.com/Altinity/ClickHouse into 2091-support-firstafter-keywords-for-alter-table-modify-column-in-iceberg-tables
CI triageNot caused by this PR. Iceberg
Approve from a #2289 perspective. Optional: rerun integration 3/8 and 4/8; fix settings snapshots in clickhouse-regression separately. |
|
Tests in Altinity/clickhouse-regression@a7fcec8 for FIRST / AFTER are passing on Glue and Ice rest catalogs. LGTM |
…-alter-table-modify-column-in-iceberg-tables
…-alter-table-modify-column-in-iceberg-tables
|
Same error at lines 140, 234, 251, and 264. void generateAddColumnMetadata(const String & column_name, DataTypePtr type, bool first = false, const String & after_column = {});
void generateAddColumnMetadata(const String & column_name, DataTypePtr type);The new overload's default arguments make a two-argument call match the old declaration as well. The @subkanthi Maybe drop the two-argument declaration and keep the one with the defaults? The existing tests then resolve to that function. The other builds on this run passed because they do not compile |
PR #2289 CI TriageRun 35752871749, Summary
Nothing in the remaining list matches this diff (Iceberg Infrastructure
Pre-existing flaky
UnknownThese block a clean approval until the branch rate is checked. None of them is exercised by this diff.
|
…-alter-table-modify-column-in-iceberg-tables
|
Run 35915202409 on The remaining red checks are outside this PR:
LGTM, just waiting on dev review process |
|
AI audit note: This review was generated by AI (gpt-6-sol). The analysis is static; no tests or fault-injection runs were performed locally. Audit update for PR #2289 (Iceberg Confirmed defectsHigh: A retry of position-only Medium: Positioning bypasses rejection of an unrecordable type change. Medium: Coverage summary
|
…on and self-`AFTER` - `isModifyColumnApplied` no longer dereferences a null type for a position-only `MODIFY COLUMN c FIRST`, and it verifies the requested `FIRST`/`AFTER` position before a retry after an unknown commit outcome reports success. - A primitive type change that Iceberg cannot record (e.g. `Int32` -> `UInt32`) is rejected even when combined with `FIRST`/`AFTER`. - `MODIFY COLUMN b ... AFTER b` keeps the column in place instead of throwing, matching `ColumnsDescription::modifyColumnOrder`. Signed-off-by: Kanthi Subramanian <subkanthi@gmail.com>
|
I added test coverage for the aforementioned defects. Will test fixes once they are pushed and the build is available. |
…. FIRST/AFTER` `generateModifyColumnMetadata` could rebuild the current ClickHouse type only for primitive Iceberg fields. For a complex field it could not tell a restated type from a change Iceberg cannot record, so `MODIFY COLUMN t Tuple(a UInt32) FIRST` on `Tuple(a Int32)` silently dropped the type change and only moved the column. Rebuild the type for every field with the reader's own conversion, exposed as `IcebergSchemaProcessor::getClickHouseFieldType`. A differing type that maps to the same Iceberg type is now rejected with or without positioning, and restating the exact same complex type is a no-op (or a pure reposition) like for primitive types. Related: #2289 Signed-off-by: Kanthi Subramanian <subkanthi@gmail.com>
closes: #2091
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Added support for the
FIRSTandAFTERclauses withALTER TABLE ADD COLUMNandALTER TABLE MODIFY COLUMNfor Iceberg tables (DataLakeCatalog).CI/CD Options
Exclude tests:
Regression jobs to run: