[fix](build index)Fix build index failed on renamed column (#42882)

Add `column_unique_ids` in `TOlapTableIndex` thrift struct, to get col_unique_id when building index.
apache · Nov 1, 2024 · 01a3c41 · 01a3c41
1 parent f4f8229
commit 01a3c41
Show file tree

Hide file tree

Showing 15 changed files with 73 additions and 19 deletions.
diff --git a/be/src/olap/tablet_schema.cpp b/be/src/olap/tablet_schema.cpp
@@ -749,7 +749,15 @@ void TabletIndex::init_from_thrift(const TOlapTableIndex& index,
         if (column_idx >= 0) {
             col_unique_ids[i] = tablet_schema.column(column_idx).unique_id();
         } else {
-            col_unique_ids[i] = -1;
+            // if column unique id not found by column name, find by column unique id
+            // column unique id can not bigger than tablet schema column size, if bigger than column size means
+            // this column is a new column added by light schema change
+            if (index.__isset.column_unique_ids &&
+                index.column_unique_ids[i] < tablet_schema.num_columns()) {
+                col_unique_ids[i] = index.column_unique_ids[i];
+            } else {
+                col_unique_ids[i] = -1;
+            }
         }
     }
     _col_unique_ids = std::move(col_unique_ids);

diff --git a/be/src/olap/task/index_builder.cpp b/be/src/olap/task/index_builder.cpp
@@ -363,10 +363,14 @@ Status IndexBuilder::handle_single_rowset(RowsetMetaSharedPtr output_rowset_meta
                 auto column_name = inverted_index.columns[0];
                 auto column_idx = output_rowset_schema->field_index(column_name);
                 if (column_idx < 0) {
-                    LOG(WARNING) << "referenced column was missing. "
-                                 << "[column=" << column_name << " referenced_column=" << column_idx
-                                 << "]";
-                    continue;
+                    column_idx =
+                            output_rowset_schema->field_index(inverted_index.column_unique_ids[0]);
+                    if (column_idx < 0) {
+                        LOG(WARNING) << "referenced column was missing. "
+                                     << "[column=" << column_name
+                                     << " referenced_column=" << column_idx << "]";
+                        continue;
+                    }
                 }
                 auto column = output_rowset_schema->column(column_idx);
                 if (!InvertedIndexColumnWriter::check_support_inverted_index(column)) {

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
@@ -2082,6 +2082,7 @@ public int getAsInt() {
                             index.setIndexId(existedIdx.getIndexId());
                             index.setColumns(existedIdx.getColumns());
                             index.setProperties(existedIdx.getProperties());
+                            index.setColumnUniqueIds(existedIdx.getColumnUniqueIds());
                             if (indexDef.getPartitionNames().isEmpty()) {
                                 invertedIndexOnPartitions.put(index.getIndexId(), olapTable.getPartitionNames());
                             } else {
@@ -2682,6 +2683,7 @@ private boolean processAddIndex(CreateIndexClause alterClause, OlapTable olapTab
             if (column != null) {
                 indexDef.checkColumn(column, olapTable.getKeysType(),
                         olapTable.getTableProperty().getEnableUniqueKeyMergeOnWrite());
+                indexDef.getColumnUniqueIds().add(column.getUniqueId());
             } else {
                 throw new DdlException("index column does not exist in table. invalid column: " + col);
             }
@@ -2692,6 +2694,7 @@ private boolean processAddIndex(CreateIndexClause alterClause, OlapTable olapTab
         // so here update column name in CreateIndexClause after checkColumn for indexDef,
         // there will use the column name in olapTable insead of the column name in CreateIndexClause.
         alterIndex.setColumns(indexDef.getColumns());
+        alterIndex.setColumnUniqueIds(indexDef.getColumnUniqueIds());
         newIndexes.add(alterIndex);
         return false;
     }

diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/BuildIndexClause.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/BuildIndexClause.java
@@ -73,7 +73,7 @@ public void analyze(Analyzer analyzer) throws AnalysisException {
         indexDef.analyze();
         this.index = new Index(Env.getCurrentEnv().getNextId(), indexDef.getIndexName(),
                 indexDef.getColumns(), indexDef.getIndexType(),
-                indexDef.getProperties(), indexDef.getComment());
+                indexDef.getProperties(), indexDef.getComment(), indexDef.getColumnUniqueIds());
     }
 
     @Override

diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateIndexClause.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateIndexClause.java
@@ -73,7 +73,7 @@ public void analyze(Analyzer analyzer) throws AnalysisException {
         indexDef.analyze();
         this.index = new Index(Env.getCurrentEnv().getNextId(), indexDef.getIndexName(),
                 indexDef.getColumns(), indexDef.getIndexType(),
-                indexDef.getProperties(), indexDef.getComment());
+                indexDef.getProperties(), indexDef.getComment(), indexDef.getColumnUniqueIds());
     }
 
     @Override

diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java
@@ -592,7 +592,8 @@ public void analyze(Analyzer analyzer) throws UserException {
                     }
                 }
                 indexes.add(new Index(Env.getCurrentEnv().getNextId(), indexDef.getIndexName(), indexDef.getColumns(),
-                        indexDef.getIndexType(), indexDef.getProperties(), indexDef.getComment()));
+                        indexDef.getIndexType(), indexDef.getProperties(), indexDef.getComment(),
+                        indexDef.getColumnUniqueIds()));
                 distinct.add(indexDef.getIndexName());
                 distinctCol.add(Pair.of(indexDef.getIndexType(),
                         indexDef.getColumns().stream().map(String::toUpperCase).collect(Collectors.toList())));

diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/IndexDef.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/IndexDef.java
@@ -42,6 +42,7 @@ public class IndexDef {
     private Map<String, String> properties;
     private boolean isBuildDeferred = false;
     private PartitionNames partitionNames;
+    private List<Integer> columnUniqueIds = Lists.newArrayList();
 
     public static final String NGRAM_SIZE_KEY = "gram_size";
     public static final String NGRAM_BF_SIZE_KEY = "bf_size";
@@ -196,6 +197,10 @@ public List<String> getPartitionNames() {
         return partitionNames == null ? Lists.newArrayList() : partitionNames.getPartitionNames();
     }
 
+    public List<Integer> getColumnUniqueIds() {
+        return columnUniqueIds;
+    }
+
     public enum IndexType {
         BITMAP,
         INVERTED,

diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/Index.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/Index.java
@@ -48,7 +48,7 @@
 
 /**
  * Internal representation of index, including index type, name, columns and comments.
- * This class will used in olaptable
+ * This class will be used in olap table
  */
 public class Index implements Writable {
     public static final int INDEX_ID_INIT_VALUE = -1;
@@ -65,15 +65,19 @@ public class Index implements Writable {
     private Map<String, String> properties;
     @SerializedName(value = "ct", alternate = {"comment"})
     private String comment;
+    @SerializedName(value = "cui", alternate = {"columnUniqueIds"})
+    private List<Integer> columnUniqueIds;
 
     public Index(long indexId, String indexName, List<String> columns,
-            IndexDef.IndexType indexType, Map<String, String> properties, String comment) {
+            IndexDef.IndexType indexType, Map<String, String> properties, String comment,
+            List<Integer> columnUniqueIds) {
         this.indexId = indexId;
         this.indexName = indexName;
         this.columns = columns == null ? Lists.newArrayList() : Lists.newArrayList(columns);
         this.indexType = indexType;
         this.properties = properties == null ? Maps.newHashMap() : Maps.newHashMap(properties);
         this.comment = comment;
+        this.columnUniqueIds = columnUniqueIds == null ? Lists.newArrayList() : Lists.newArrayList(columnUniqueIds);
         if (indexType == IndexDef.IndexType.INVERTED) {
             if (this.properties != null && !this.properties.isEmpty()) {
                 if (this.properties.containsKey(InvertedIndexUtil.INVERTED_INDEX_PARSER_KEY)) {
@@ -97,6 +101,7 @@ public Index() {
         this.indexType = null;
         this.properties = null;
         this.comment = null;
+        this.columnUniqueIds = null;
     }
 
     public long getIndexId() {
@@ -186,6 +191,14 @@ public void setComment(String comment) {
         this.comment = comment;
     }
 
+    public List<Integer> getColumnUniqueIds() {
+        return columnUniqueIds;
+    }
+
+    public void setColumnUniqueIds(List<Integer> columnUniqueIds) {
+        this.columnUniqueIds = columnUniqueIds;
+    }
+
     @Override
     public void write(DataOutput out) throws IOException {
         Text.writeString(out, GsonUtils.GSON.toJson(this));
@@ -203,7 +216,7 @@ public int hashCode() {
 
     public Index clone() {
         return new Index(indexId, indexName, new ArrayList<>(columns),
-                indexType, new HashMap<>(properties), comment);
+                indexType, new HashMap<>(properties), comment, columnUniqueIds);
     }
 
     @Override
@@ -247,6 +260,7 @@ public TOlapTableIndex toThrift() {
         if (properties != null) {
             tIndex.setProperties(properties);
         }
+        tIndex.setColumnUniqueIds(columnUniqueIds);
         return tIndex;
     }
 

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
@@ -387,6 +387,13 @@ public void initSchemaColumnUniqueId() {
         maxColUniqueId = Column.COLUMN_UNIQUE_ID_INIT_VALUE;
         this.schema.forEach(column -> {
             column.setUniqueId(incAndGetMaxColUniqueId());
+            this.indexes.forEach(index -> {
+                index.getColumns().forEach(col -> {
+                    if (col.equalsIgnoreCase(column.getName())) {
+                        index.getColumnUniqueIds().add(column.getUniqueId());
+                    }
+                });
+            });
             if (LOG.isDebugEnabled()) {
                 LOG.debug("indexId: {},  column:{}, uniqueId:{}",
                         indexId, column, column.getUniqueId());

diff --git a/...ore/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/IndexDefinition.java b/...ore/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/IndexDefinition.java
@@ -207,6 +207,6 @@ public IndexType getIndexType() {
 
     public Index translateToCatalogStyle() {
         return new Index(Env.getCurrentEnv().getNextId(), name, cols, indexType, properties,
-                comment);
+                comment, null);
     }
 }
diff --git a/fe/fe-core/src/test/java/org/apache/doris/catalog/OlapTableTest.java b/fe/fe-core/src/test/java/org/apache/doris/catalog/OlapTableTest.java
@@ -59,7 +59,7 @@ int getCurrentEnvJournalVersion() {
             }
             OlapTable tbl = (OlapTable) table;
             tbl.setIndexes(Lists.newArrayList(new Index(0, "index", Lists.newArrayList("col"),
-                    IndexDef.IndexType.BITMAP, null, "xxxxxx")));
+                    IndexDef.IndexType.BITMAP, null, "xxxxxx", Lists.newArrayList(1))));
             System.out.println("orig table id: " + tbl.getId());
 
             FastByteArrayOutputStream byteArrayOutputStream = new FastByteArrayOutputStream();

diff --git a/fe/fe-core/src/test/java/org/apache/doris/common/proc/IndexesProcNodeTest.java b/fe/fe-core/src/test/java/org/apache/doris/common/proc/IndexesProcNodeTest.java
@@ -42,18 +42,18 @@ public class IndexesProcNodeTest {
     public void testFetchResult() throws AnalysisException {
         List<Index> indexes = new ArrayList<>();
         Index indexBitmap = new Index(1, "bitmap_index", Lists.newArrayList("col_1"),
-                IndexType.BITMAP, null, "bitmap index on col_1");
+                IndexType.BITMAP, null, "bitmap index on col_1", Lists.newArrayList(1));
         Map<String, String> invertedProperties = new HashMap<>();
         invertedProperties.put("parser", "unicode");
         Index indexInverted = new Index(2, "inverted_index", Lists.newArrayList("col_2"),
-                        IndexType.INVERTED, invertedProperties, "inverted index on col_2");
+                        IndexType.INVERTED, invertedProperties, "inverted index on col_2", Lists.newArrayList(2));
         Index indexBf = new Index(3, "bloomfilter_index", Lists.newArrayList("col_3"),
-                IndexType.BLOOMFILTER, null, "bloomfilter index on col_3");
+                IndexType.BLOOMFILTER, null, "bloomfilter index on col_3", Lists.newArrayList(3));
         Map<String, String> ngramProperties = new HashMap<>();
         ngramProperties.put("gram_size", "3");
         ngramProperties.put("bf_size", "256");
         Index indexNgramBf = new Index(4, "ngram_bf_index", Lists.newArrayList("col_4"),
-                        IndexType.NGRAM_BF, ngramProperties, "ngram_bf index on col_4");
+                        IndexType.NGRAM_BF, ngramProperties, "ngram_bf index on col_4", Lists.newArrayList(4));
         indexes.add(indexBitmap);
         indexes.add(indexInverted);
         indexes.add(indexBf);

diff --git a/fe/fe-core/src/test/java/org/apache/doris/persist/TableAddOrDropColumnsInfoTest.java b/fe/fe-core/src/test/java/org/apache/doris/persist/TableAddOrDropColumnsInfoTest.java
@@ -65,7 +65,7 @@ public void testSerialization() throws IOException {
         indexSchemaMap.put(tableId, fullSchema);
 
         List<Index> indexes = Lists.newArrayList(
-                new Index(0, "index", Lists.newArrayList("testCol1"), IndexDef.IndexType.INVERTED, null, "xxxxxx"));
+                new Index(0, "index", Lists.newArrayList("testCol1"), IndexDef.IndexType.INVERTED, null, "xxxxxx", Lists.newArrayList(1)));
 
         TableAddOrDropColumnsInfo tableAddOrDropColumnsInfo1 = new TableAddOrDropColumnsInfo("", dbId, tableId,
                 indexSchemaMap, indexes, jobId);

diff --git a/gensrc/thrift/Descriptors.thrift b/gensrc/thrift/Descriptors.thrift
@@ -228,6 +228,7 @@ struct TOlapTableIndex {
   4: optional string comment
   5: optional i64 index_id
   6: optional map<string, string> properties
+  7: optional list<i32> column_unique_ids
 }
 
 struct TOlapTableIndexSchema {

diff --git a/...ion-test/suites/inverted_index_p0/index_change/test_index_change_on_renamed_column.groovy b/...ion-test/suites/inverted_index_p0/index_change/test_index_change_on_renamed_column.groovy
@@ -18,6 +18,10 @@
 import org.codehaus.groovy.runtime.IOGroovyMethods
 
 suite("test_index_change_on_renamed_column") {
+    def backendId_to_backendIP = [:]
+    def backendId_to_backendHttpPort = [:]
+    getBackendIpHttpPort(backendId_to_backendIP, backendId_to_backendHttpPort);
+
     def timeout = 60000
     def delta_time = 1000
     def alter_res = "null"
@@ -67,7 +71,7 @@ suite("test_index_change_on_renamed_column") {
             `id` INT COMMENT "",
             `s` STRING COMMENT ""
         )
-        DUPLICATE KEY(`id`) DISTRIBUTED BY HASH(`id`)
+        DUPLICATE KEY(`id`) DISTRIBUTED BY HASH(`id`) BUCKETS 1
         PROPERTIES ( "replication_num" = "1" );
         """
 
@@ -101,6 +105,13 @@ suite("test_index_change_on_renamed_column") {
     qt_select2 """ SELECT * FROM ${tableName} order by id; """
     qt_select3 """ SELECT * FROM ${tableName} where s1 match 'welcome'; """
 
+    def tablets = sql_return_maparray """ show tablets from ${tableName}; """
+    String tablet_id = tablets[0].TabletId
+    String backend_id = tablets[0].BackendId
+    String ip = backendId_to_backendIP.get(backend_id)
+    String port = backendId_to_backendHttpPort.get(backend_id)
+    check_nested_index_file(ip, port, tablet_id, 3, 1, "V2")
+
     // drop inverted index on renamed column
     sql """ alter table ${tableName} drop index idx_s; """
     wait_for_latest_op_on_table_finish(tableName, timeout)