morningman opened a new pull request, #69471:
URL: https://github.com/apache/doris/pull/69471
### What problem does this PR solve?
Issue Number: None
Related PR: #65329, #64304
Problem Summary:
**In short.** This restores a rule that #65329 introduced and that
branch-4.1 and branch-4.2 still follow: on an Iceberg format-version 3 table,
`ALTER TABLE ... ADD COLUMN` and `RENAME COLUMN` may not name a top-level
column `_row_id` or `_last_updated_sequence_number`. Master lost the ALTER half
of the rule when #64304 moved Iceberg DDL into the connector plugin.
**Background.** Doris exposes the row lineage of a format-version 3 table as
two hidden columns, `_row_id` and `_last_updated_sequence_number`, next to the
table's own columns. #65329 (merged 07-23) rejected those names for the columns
of a v3 table, in CREATE TABLE and in ALTER TABLE. #64304 (merged 07-30) was
developed in parallel and rewrote Iceberg DDL in `fe-connector-iceberg`. It
kept the CREATE TABLE check
(`IcebergConnectorMetadata.rejectReservedRowLineageColumns`), but ALTER only
checks `_file` and `_pos`. Iceberg itself does not check the names in a schema
update.
**The problem, and what it cost.** On master these statements commit on a v3
table. Doris then appends the hidden column to a schema that already has a
column with the same name. With the default metadata cache, `DESC` and every
query on the table fail with `Duplicate column name found: _row_id`. With the
table cache disabled, the user column hides the row lineage column instead.
Spark cannot read such a table either: it fails with `Table column names
conflict with names reserved for Iceberg metadata columns`.
| On a v3 table | Master today | After this PR (same as branch-4.2) |
|---|---|---|
| `ADD COLUMN _row_id BIGINT` | commits; the table can no longer be read |
`Cannot add Iceberg v3 reserved row lineage column: _row_id` |
| `ADD COLUMN (extra INT, _LAST_UPDATED_SEQUENCE_NUMBER BIGINT)` | commits
both columns | rejected; nothing is committed |
| `RENAME COLUMN c _last_updated_sequence_number` | commits | `Cannot rename
to Iceberg v3 reserved row lineage column: _last_updated_sequence_number` |
| the same names on a v2 table, or on a nested field such as `s._row_id` |
allowed | allowed |
**How this PR fixes it.** `CatalogBackedIcebergCatalogOps.addColumn`,
`addColumns` and `renameColumn` call a new
`IcebergWriterHelper.validateNotRowLineageColumn` after they load the table,
next to the existing VARIANT format check, so the rule reads the table's real
format version. The check ignores case and takes the names from Iceberg's
`MetadataColumns`. The messages are the ones branch-4.2 uses.
One difference from branch-4.2 is deliberate. branch-4.2 also rejects
dropping such a column or renaming it away. But Iceberg lets such a column
exist on a v3 table: Spark adds one with `ADD COLUMN`, and a v2 table that has
one can be upgraded to v3. After that neither Spark nor Doris can read the
table, and dropping the column or renaming it away is how it becomes readable
again. This PR keeps both allowed. `MODIFY COLUMN`, a column comment or a
reorder that names `_row_id` already fails on master, because the row lineage
columns are not in the Iceberg schema.
**Results.**
- Reproduced on a local cluster with a `file://` v3 table. Before: `ADD
COLUMN _row_id BIGINT` and `RENAME COLUMN c _last_updated_sequence_number`
commit, and `DESC` then fails with `Duplicate column name found: _row_id`.
After: both fail with the messages above, and the table stays readable. A table
that Spark upgraded from v2 with a `_row_id` column becomes readable in Doris
and in Spark after `DROP COLUMN _row_id` in Doris, and so does one where Spark
added the column, after a rename.
- New tests in `CatalogBackedIcebergCatalogOpsColumnEvolutionTest` run
against a real in-memory Iceberg catalog. On a v3 table the three statements
fail and leave the schema unchanged. v2 tables and nested fields may use the
names. An existing column with such a name can be dropped or renamed away. The
first test fails without this change. All 1640 tests of fe-connector-iceberg
pass.
- `test_iceberg_v3_row_lineage_query_insert` now runs the three statements
on its v3 table, for Parquet and ORC, and checks the errors. The rest of the
suite then writes to and reads the same table. With the connector built without
this change, the suite fails at the first of the three.
### Release note
Fix: On an Iceberg format-version 3 table, `ALTER TABLE ... ADD COLUMN` and
`RENAME COLUMN` now fail when they would name a top-level column `_row_id` or
`_last_updated_sequence_number`. Before, the statement succeeded and left the
table unreadable.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- [x] Unit Test
- [x] Manual test (add detailed scripts or steps below)
On a local cluster with a `file://` Iceberg catalog: `CREATE TABLE t
(id INT, c STRING) PROPERTIES ('format-version'='3')`, then `ALTER TABLE t ADD
COLUMN _row_id BIGINT` and `ALTER TABLE t RENAME COLUMN c
_last_updated_sequence_number` fail and `DESC t` still works. A Spark table
upgraded from v2 with a `_row_id` column becomes readable after `ALTER TABLE
... DROP COLUMN _row_id` in Doris.
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
These ALTER statements now fail on format-version 3 tables. Dropping
such a column or renaming it away stays allowed, unlike branch-4.2.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
