diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java index 8396e36bdb7f16..64e18ad273266d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java +++ b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java @@ -70,6 +70,7 @@ import org.apache.doris.common.UserException; import org.apache.doris.common.util.BufferSizeUtil; import org.apache.doris.common.util.DbUtil; +import org.apache.doris.common.util.DebugPointUtil; import org.apache.doris.common.util.DynamicPartitionUtil; import org.apache.doris.common.util.ListComparator; import org.apache.doris.common.util.PropertyAnalyzer; @@ -3731,6 +3732,18 @@ public void updateBaseIndexSchema(OlapTable olapTable, Map indexSchema = indexSchemaMap.get(indexIds.get(i)); MaterializedIndexMeta currentIndexMeta = olapTable.getIndexMetaByIndexId(indexIds.get(i)); + String forceStaleMaxColUniqueIdTable = DebugPointUtil.getDebugParamOrDefault( + "FE.SchemaChangeHandler.updateBaseIndexSchema.forceStaleMaxColUniqueId", "table_name", ""); + if (olapTable.getName().equals(forceStaleMaxColUniqueIdTable)) { + currentIndexMeta.setMaxColUniqueId(Column.COLUMN_UNIQUE_ID_INIT_VALUE); + } + + // Preserve unique ids from the old schema before replacing it. Otherwise a stale maxColUniqueId can + // forget the highest id when its column is dropped, allowing a later column to reuse that id. + int maxColUniqueId = currentIndexMeta.getMaxColUniqueId(); + for (Column column : currentIndexMeta.getSchema()) { + maxColUniqueId = Math.max(maxColUniqueId, column.getUniqueId()); + } currentIndexMeta.setSchema(indexSchema); int currentSchemaVersion = currentIndexMeta.getSchemaVersion(); @@ -3738,11 +3751,8 @@ public void updateBaseIndexSchema(OlapTable olapTable, Map maxColUniqueId) { - maxColUniqueId = column.getUniqueId(); - } + maxColUniqueId = Math.max(maxColUniqueId, column.getUniqueId()); } currentIndexMeta.setMaxColUniqueId(maxColUniqueId); currentIndexMeta.setIndexes(indexes); diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/MaterializedIndexMeta.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/MaterializedIndexMeta.java index dca0f1d8234b56..297aea722d93a3 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/MaterializedIndexMeta.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/MaterializedIndexMeta.java @@ -279,6 +279,7 @@ public void gsonPostProcess() throws IOException { if (sessionVariables == null) { sessionVariables = Maps.newHashMap(); } + refreshMaxColUniqueId(); initColumnNameMap(); } @@ -349,7 +350,7 @@ public int incAndGetMaxColUniqueId() { } public int getMaxColUniqueId() { - return this.maxColUniqueId; + return Math.max(maxColUniqueId, getSchemaMaxColUniqueId()); } public void setMaxColUniqueId(int maxColUniqueId) { @@ -367,6 +368,18 @@ public void initSchemaColumnUniqueId() { }); } + private void refreshMaxColUniqueId() { + maxColUniqueId = getMaxColUniqueId(); + } + + private int getSchemaMaxColUniqueId() { + int schemaMaxColUniqueId = Column.COLUMN_UNIQUE_ID_INIT_VALUE; + for (Column column : schema) { + schemaMaxColUniqueId = Math.max(schemaMaxColUniqueId, column.getUniqueId()); + } + return schemaMaxColUniqueId; + } + public void initColumnNameMap() { // case insensitive nameToColumn = Maps.newTreeMap(String.CASE_INSENSITIVE_ORDER); diff --git a/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeHandlerTest.java b/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeHandlerTest.java index 8839fca5a54e1e..283e2f7f90b309 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeHandlerTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeHandlerTest.java @@ -844,6 +844,43 @@ public void testAggAddOrDropColumn() throws Exception { } } + @Test + public void testReaddDroppedValueColumnUsesNewUniqueId() throws Exception { + String tableName = "sc_dup_readd_value_column"; + dropTable("test." + tableName, false); + createTable("CREATE TABLE test." + tableName + " (\n" + + "event_time DATETIME NOT NULL,\n" + + "user_id BIGINT NOT NULL,\n" + + "item_id INT NOT NULL,\n" + + "amount DECIMAL(10, 2) NOT NULL,\n" + + "city VARCHAR(64) NOT NULL\n" + + ") DUPLICATE KEY(event_time, user_id)\n" + + "DISTRIBUTED BY HASH(user_id) BUCKETS 1\n" + + "PROPERTIES ('replication_num' = '1', 'light_schema_change' = 'true')"); + + Database db = Env.getCurrentInternalCatalog().getDbOrMetaException("test"); + OlapTable tbl = (OlapTable) db.getTableOrMetaException(tableName, Table.TableType.OLAP); + MaterializedIndexMeta indexMeta = tbl.getIndexMetaByIndexId(tbl.getBaseIndexId()); + int droppedColumnUniqueId = tbl.getColumn("city").getUniqueId(); + + tbl.writeLock(); + try { + indexMeta.setMaxColUniqueId(Column.COLUMN_UNIQUE_ID_INIT_VALUE); + } finally { + tbl.writeUnlock(); + } + + alterTable("ALTER TABLE test." + tableName + " DROP COLUMN city", connectContext); + jobSize++; + waitAlterJobDone(Env.getCurrentEnv().getSchemaChangeHandler().getAlterJobsV2()); + Assertions.assertTrue(indexMeta.getMaxColUniqueId() >= droppedColumnUniqueId); + + alterTable("ALTER TABLE test." + tableName + " ADD COLUMN city VARCHAR(64)", connectContext); + jobSize++; + waitAlterJobDone(Env.getCurrentEnv().getSchemaChangeHandler().getAlterJobsV2()); + Assertions.assertTrue(tbl.getColumn("city").getUniqueId() > droppedColumnUniqueId); + } + @Test public void testUniqAddOrDropColumn() throws Exception { diff --git a/fe/fe-core/src/test/java/org/apache/doris/catalog/MaterializedIndexMetaTest.java b/fe/fe-core/src/test/java/org/apache/doris/catalog/MaterializedIndexMetaTest.java new file mode 100644 index 00000000000000..4e12aca4e3a5a3 --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/catalog/MaterializedIndexMetaTest.java @@ -0,0 +1,64 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.catalog; + +import org.apache.doris.persist.gson.GsonUtils; +import org.apache.doris.thrift.TStorageType; + +import com.google.common.collect.Lists; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class MaterializedIndexMetaTest { + + @Test + public void testDeserializeRepairsStaleMaxColUniqueId() throws Exception { + MaterializedIndexMeta meta = createMeta(); + meta.setMaxColUniqueId(Column.COLUMN_UNIQUE_ID_INIT_VALUE); + + String json = GsonUtils.GSON.toJson(meta); + MaterializedIndexMeta replayedMeta = GsonUtils.GSON.fromJson(json, MaterializedIndexMeta.class); + + Assertions.assertEquals(7, replayedMeta.getMaxColUniqueId()); + replayedMeta.setSchema(Lists.newArrayList(replayedMeta.getSchema().get(0))); + Assertions.assertEquals(8, replayedMeta.incAndGetMaxColUniqueId()); + } + + @Test + public void testDeserializePreservesHigherMaxColUniqueId() throws Exception { + MaterializedIndexMeta meta = createMeta(); + meta.setMaxColUniqueId(11); + + String json = GsonUtils.GSON.toJson(meta); + MaterializedIndexMeta replayedMeta = GsonUtils.GSON.fromJson(json, MaterializedIndexMeta.class); + + replayedMeta.setSchema(Lists.newArrayList(replayedMeta.getSchema().get(0))); + Assertions.assertEquals(11, replayedMeta.getMaxColUniqueId()); + Assertions.assertEquals(12, replayedMeta.incAndGetMaxColUniqueId()); + } + + private MaterializedIndexMeta createMeta() { + Column keyColumn = new Column("k", PrimitiveType.INT); + keyColumn.setUniqueId(0); + Column valueColumn = new Column("v", PrimitiveType.INT); + valueColumn.setUniqueId(7); + return new MaterializedIndexMeta( + 1L, Lists.newArrayList(keyColumn, valueColumn), 1, 1, (short) 1, + TStorageType.COLUMN, KeysType.DUP_KEYS, null); + } +} diff --git a/regression-test/data/fault_injection_p0/test_readd_dropped_column_unique_id.out b/regression-test/data/fault_injection_p0/test_readd_dropped_column_unique_id.out new file mode 100644 index 00000000000000..e22313ec364596 --- /dev/null +++ b/regression-test/data/fault_injection_p0/test_readd_dropped_column_unique_id.out @@ -0,0 +1,8 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !old_row -- +1 \N + +-- !all_rows -- +1 \N +2 67890 + diff --git a/regression-test/suites/fault_injection_p0/test_readd_dropped_column_unique_id.groovy b/regression-test/suites/fault_injection_p0/test_readd_dropped_column_unique_id.groovy new file mode 100644 index 00000000000000..777e17ef682e9e --- /dev/null +++ b/regression-test/suites/fault_injection_p0/test_readd_dropped_column_unique_id.groovy @@ -0,0 +1,67 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import org.apache.doris.regression.suite.ClusterOptions + +suite("test_readd_dropped_column_unique_id", "docker") { + def options = new ClusterOptions() + options.feConfigs += ["enable_debug_points=true"] + options.cloudMode = false + + docker(options) { + sql "DROP TABLE IF EXISTS test_readd_dropped_column_unique_id" + sql """ + CREATE TABLE test_readd_dropped_column_unique_id ( + k INT NOT NULL, + v INT NULL + ) + DUPLICATE KEY(k) + DISTRIBUTED BY HASH(k) BUCKETS 1 + PROPERTIES ( + "replication_num" = "1", + "light_schema_change" = "true", + "disable_auto_compaction" = "true" + ) + """ + sql "INSERT INTO test_readd_dropped_column_unique_id VALUES (1, 12345)" + sql "SYNC" + + def debugPoint = "FE.SchemaChangeHandler.updateBaseIndexSchema.forceStaleMaxColUniqueId" + try { + GetDebugPoint().enableDebugPointForAllFEs( + debugPoint, [table_name: "test_readd_dropped_column_unique_id"]) + sql "ALTER TABLE test_readd_dropped_column_unique_id DROP COLUMN v" + } finally { + GetDebugPoint().disableDebugPointForAllFEs(debugPoint) + } + + sql "ALTER TABLE test_readd_dropped_column_unique_id ADD COLUMN v INT NULL" + + order_qt_old_row """ + SELECT k, v + FROM test_readd_dropped_column_unique_id + ORDER BY k + """ + + sql "INSERT INTO test_readd_dropped_column_unique_id VALUES (2, 67890)" + order_qt_all_rows """ + SELECT k, v + FROM test_readd_dropped_column_unique_id + ORDER BY k + """ + } +}