This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 87156eb714c [fix](fe) Reject Iceberg v3 row lineage names in ALTER ADD
and RENAME COLUMN (#69471)
87156eb714c is described below
commit 87156eb714c19761f41e0d5ef2aff2c55cd5f915
Author: Mingyu Chen (Rayner) <[email protected]>
AuthorDate: Fri Oct 23 09:04:21 2026 +0100
[fix](fe) Reject Iceberg v3 row lineage names in ALTER ADD and RENAME
COLUMN (#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.
---
.../doris/connector/iceberg/IcebergCatalogOps.java | 3 +
.../connector/iceberg/IcebergWriterHelper.java | 17 ++++++
...BackedIcebergCatalogOpsColumnEvolutionTest.java | 68 ++++++++++++++++++++++
...test_iceberg_v3_row_lineage_query_insert.groovy | 21 +++++++
4 files changed, 109 insertions(+)
diff --git
a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergCatalogOps.java
b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergCatalogOps.java
index 6fdd8d24206..8d63b2b0158 100644
---
a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergCatalogOps.java
+++
b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergCatalogOps.java
@@ -523,6 +523,7 @@ public interface IcebergCatalogOps {
if (column.getType().isVariantType()) {
IcebergWriterHelper.validateVariantColumn(table,
column.getName());
}
+ IcebergWriterHelper.validateNotRowLineageColumn(table,
column.getName(), "add");
IcebergNestedColumnEvolution.validateNoCaseInsensitiveSiblingCollision(
table.schema().asStruct(), "", column.getName(), null,
"add");
UpdateSchema updateSchema = table.updateSchema();
@@ -541,6 +542,7 @@ public interface IcebergCatalogOps {
if (column.getType().isVariantType()) {
IcebergWriterHelper.validateVariantColumn(table,
column.getName());
}
+ IcebergWriterHelper.validateNotRowLineageColumn(table,
column.getName(), "add");
}
IcebergNestedColumnEvolution.validateNoCaseInsensitiveTopLevelCollisions(table.schema(),
columns);
UpdateSchema updateSchema = table.updateSchema();
@@ -564,6 +566,7 @@ public interface IcebergCatalogOps {
@Override
public void renameColumn(String dbName, String tableName, String
oldName, String newName) {
withTable(dbName, tableName, table -> {
+ IcebergWriterHelper.validateNotRowLineageColumn(table,
newName, "rename to");
IcebergNestedColumnEvolution.renameTopLevelColumn(table,
oldName, newName);
return null;
});
diff --git
a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergWriterHelper.java
b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergWriterHelper.java
index 29c9ba68fae..abf2edc38f2 100644
---
a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergWriterHelper.java
+++
b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergWriterHelper.java
@@ -122,6 +122,23 @@ final class IcebergWriterHelper {
}
}
+ /**
+ * Rejects {@code columnName} as the new name of a top-level column of
{@code table} when it is a format-version
+ * 3 row-lineage column name ({@code _row_id}, {@code
_last_updated_sequence_number}, in any case). Such a table
+ * reads both as hidden columns, so a column that takes either name makes
every later read of the table fail with
+ * a duplicate column. Nested fields and older format versions may use the
names, and dropping such a column or
+ * renaming it away stays allowed: that is how a table that already holds
one becomes readable again.
+ */
+ static void validateNotRowLineageColumn(Table table, String columnName,
String operation) {
+ int formatVersion = getFormatVersion(table);
+ if (formatVersion >= ICEBERG_ROW_LINEAGE_MIN_VERSION
+ && (MetadataColumns.ROW_ID.name().equalsIgnoreCase(columnName)
+ ||
MetadataColumns.LAST_UPDATED_SEQUENCE_NUMBER.name().equalsIgnoreCase(columnName)))
{
+ throw new DorisConnectorException("Cannot " + operation + "
Iceberg v" + formatVersion
+ + " reserved row lineage column: " + columnName);
+ }
+ }
+
/**
* Converts the BE data-file commit fragments into an iceberg {@link
WriteResult} of {@link DataFile}s
* (the INSERT / OVERWRITE / MERGE-data path). A partitioned table
requires non-empty partition values per
diff --git
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/CatalogBackedIcebergCatalogOpsColumnEvolutionTest.java
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/CatalogBackedIcebergCatalogOpsColumnEvolutionTest.java
index c581b78b35c..e292b2308b7 100644
---
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/CatalogBackedIcebergCatalogOpsColumnEvolutionTest.java
+++
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/CatalogBackedIcebergCatalogOpsColumnEvolutionTest.java
@@ -413,6 +413,74 @@ public class
CatalogBackedIcebergCatalogOpsColumnEvolutionTest {
Assertions.assertEquals(Types.LongType.get(),
schema.findField("id").type());
}
+ // ---------- row lineage names ----------
+
+ /** Creates {@code db1.<name>} at {@code formatVersion} with {@code
fields} as its top-level columns. */
+ private void createTableWithFields(String name, String formatVersion,
Types.NestedField... fields) {
+ Map<String, String> props = new
HashMap<>(IcebergSchemaBuilder.buildTableProperties(Collections.emptyMap()));
+ props.put("format-version", formatVersion);
+ ops.createTable("db1", name, new Schema(fields),
PartitionSpec.unpartitioned(), null, props);
+ }
+
+ private List<String> columnNames(String tableName) {
+ return ops.loadTable("db1", tableName).schema().columns().stream()
+ .map(Types.NestedField::name).collect(Collectors.toList());
+ }
+
+ @Test
+ public void testRejectsRowLineageNamesOnFormatThreeTable() {
+ // A format-version 3 table reads _row_id and
_last_updated_sequence_number as hidden columns, so a column
+ // that takes either name makes every later read of the table fail
with a duplicate column.
+ createTableWithFields("v3", "3", Types.NestedField.optional(1, "id",
Types.LongType.get()),
+ Types.NestedField.optional(2, "age", Types.IntegerType.get()));
+ DorisConnectorException add =
Assertions.assertThrows(DorisConnectorException.class,
+ () -> ops.addColumn("db1", "v3", change("_row_id",
Types.LongType.get(), null, true), null));
+ Assertions.assertEquals("Cannot add Iceberg v3 reserved row lineage
column: _row_id", add.getMessage());
+ DorisConnectorException addColumns =
Assertions.assertThrows(DorisConnectorException.class,
+ () -> ops.addColumns("db1", "v3", Arrays.asList(
+ change("extra", Types.IntegerType.get(), null, true),
+ change("_LAST_UPDATED_SEQUENCE_NUMBER",
Types.LongType.get(), null, true))));
+ Assertions.assertEquals("Cannot add Iceberg v3 reserved row lineage
column: _LAST_UPDATED_SEQUENCE_NUMBER",
+ addColumns.getMessage());
+ DorisConnectorException rename =
Assertions.assertThrows(DorisConnectorException.class,
+ () -> ops.renameColumn("db1", "v3", "age",
"_last_updated_sequence_number"));
+ Assertions.assertEquals(
+ "Cannot rename to Iceberg v3 reserved row lineage column:
_last_updated_sequence_number",
+ rename.getMessage());
+ Assertions.assertEquals(Arrays.asList("id", "age"), columnNames("v3"));
+ }
+
+ @Test
+ public void testAllowsRowLineageNamesOnFormatTwoTableAndNestedFields() {
+ // The names are reserved only for the top-level columns of a
format-version 3 table.
+ createTableWithFields("v2", "2", Types.NestedField.optional(1, "id",
Types.LongType.get()));
+ ops.addColumn("db1", "v2", change("_row_id", Types.LongType.get(),
null, true), null);
+ ops.renameColumn("db1", "v2", "id", "_last_updated_sequence_number");
+ Assertions.assertEquals(Arrays.asList("_last_updated_sequence_number",
"_row_id"), columnNames("v2"));
+
+ createTableWithFields("v3_nested", "3", Types.NestedField.optional(1,
"s", Types.StructType.of(
+ Types.NestedField.optional(2, "a", Types.IntegerType.get()))));
+ ops.addNestedColumn("db1", "v3_nested",
ConnectorColumnPath.of(Arrays.asList("s", "_row_id")),
+ change("_row_id", Types.LongType.get(), null, true), null);
+ ops.renameNestedColumn("db1", "v3_nested",
ConnectorColumnPath.of(Arrays.asList("s", "a")),
+ "_last_updated_sequence_number");
+ Assertions.assertEquals(Arrays.asList("_last_updated_sequence_number",
"_row_id"),
+ ops.loadTable("db1",
"v3_nested").schema().findField("s").type().asStructType().fields().stream()
+
.map(Types.NestedField::name).collect(Collectors.toList()));
+ }
+
+ @Test
+ public void
testDropsOrRenamesAwayExistingRowLineageNamesOnFormatThreeTable() {
+ // A format-version 3 table can already hold such columns, for example
one upgraded from format-version 2.
+ // Dropping them or renaming them away is how such a table becomes
readable again, so both stay allowed.
+ createTableWithFields("v3_taken", "3", Types.NestedField.optional(1,
"id", Types.LongType.get()),
+ Types.NestedField.optional(2, "_row_id", Types.LongType.get()),
+ Types.NestedField.optional(3, "_last_updated_sequence_number",
Types.LongType.get()));
+ ops.dropColumn("db1", "v3_taken", "_row_id");
+ ops.renameColumn("db1", "v3_taken", "_last_updated_sequence_number",
"seq");
+ Assertions.assertEquals(Arrays.asList("id", "seq"),
columnNames("v3_taken"));
+ }
+
// ---------- reorderColumns ----------
@Test
diff --git
a/regression-test/suites/external_table_p0/iceberg/test_iceberg_v3_row_lineage_query_insert.groovy
b/regression-test/suites/external_table_p0/iceberg/test_iceberg_v3_row_lineage_query_insert.groovy
index 4fc8d61a258..e95b779bc49 100644
---
a/regression-test/suites/external_table_p0/iceberg/test_iceberg_v3_row_lineage_query_insert.groovy
+++
b/regression-test/suites/external_table_p0/iceberg/test_iceberg_v3_row_lineage_query_insert.groovy
@@ -278,6 +278,24 @@ suite("test_iceberg_v3_row_lineage_query_insert",
"p0,external,iceberg,external_
}
}
+ // A column that takes a row lineage name would clash with the hidden
column and break every read of the table.
+ def assertAlterTableWithRowLineageColumnsFails = { tableName ->
+ test {
+ sql """alter table ${tableName} add column _row_id bigint"""
+ exception "Cannot add Iceberg v3 reserved row lineage column:
_row_id"
+ }
+
+ test {
+ sql """alter table ${tableName} add column (extra int,
_LAST_UPDATED_SEQUENCE_NUMBER bigint)"""
+ exception "Cannot add Iceberg v3 reserved row lineage column:
_LAST_UPDATED_SEQUENCE_NUMBER"
+ }
+
+ test {
+ sql """alter table ${tableName} rename column age
_last_updated_sequence_number"""
+ exception "Cannot rename to Iceberg v3 reserved row lineage
column: _last_updated_sequence_number"
+ }
+ }
+
def assertRowLineageOnlyAggregatesReadable = { tableName, expectedRowCount
->
def rowIdsWithPhysicalColumn = sql("""
select _row_id, id
@@ -434,6 +452,9 @@ suite("test_iceberg_v3_row_lineage_query_insert",
"p0,external,iceberg,external_
exception "Cannot specify invisible column
'_last_updated_sequence_number' in INSERT statement"
}
+ // The rejected ALTERs leave the table readable for the writes
and reads below.
+ assertAlterTableWithRowLineageColumnsFails(unpartitionedTable)
+
sql """insert into ${unpartitionedTable}(id, name, age) values
(4, 'Doris', 40)"""
def unpartitionedCount = sql """select count(*) from
${unpartitionedTable}"""
log.info("Checking row count after regular INSERT for
${unpartitionedTable}: result=${unpartitionedCount}")
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
