From 8ae42e1d2eb682bbbe1c5d612be3ec2179830716 Mon Sep 17 00:00:00 2001 From: Jianliang Qi Date: Wed, 30 Oct 2024 01:45:16 +0800 Subject: [PATCH] [fix](build index)Fix build index failed on renamed column --- be/src/olap/tablet_schema.cpp | 6 +++- be/src/olap/task/index_builder.cpp | 12 ++++--- .../doris/analysis/BuildIndexClause.java | 34 ++++++++++++++++++- .../doris/analysis/CreateIndexClause.java | 34 ++++++++++++++++++- .../doris/analysis/CreateTableStmt.java | 3 +- .../org/apache/doris/analysis/IndexDef.java | 10 ++++++ .../java/org/apache/doris/catalog/Index.java | 20 +++++++++-- .../plans/commands/info/IndexDefinition.java | 6 +++- .../apache/doris/catalog/OlapTableTest.java | 2 +- .../common/proc/IndexesProcNodeTest.java | 8 ++--- .../TableAddOrDropColumnsInfoTest.java | 2 +- gensrc/thrift/Descriptors.thrift | 1 + ...test_index_change_on_renamed_column.groovy | 13 ++++++- 13 files changed, 132 insertions(+), 19 deletions(-) diff --git a/be/src/olap/tablet_schema.cpp b/be/src/olap/tablet_schema.cpp index c88a23a0c360cf9..cb1079f5a286c73 100644 --- a/be/src/olap/tablet_schema.cpp +++ b/be/src/olap/tablet_schema.cpp @@ -747,7 +747,11 @@ 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 (index.__isset.column_unique_ids) { + 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 index 38a52d1d2118aa6..031748b0d791828 100644 --- 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/analysis/BuildIndexClause.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/BuildIndexClause.java index cb7ec08de78f9c8..c7f00c491820f34 100644 --- 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 @@ -18,12 +18,18 @@ package org.apache.doris.analysis; import org.apache.doris.alter.AlterOpType; +import org.apache.doris.catalog.DatabaseIf; import org.apache.doris.catalog.Env; import org.apache.doris.catalog.Index; +import org.apache.doris.catalog.OlapTable; +import org.apache.doris.catalog.TableIf; import org.apache.doris.common.AnalysisException; +import org.apache.doris.common.DdlException; import com.google.common.collect.Maps; +import java.util.ArrayList; +import java.util.List; import java.util.Map; public class BuildIndexClause extends AlterTableClause { @@ -71,9 +77,35 @@ public void analyze(Analyzer analyzer) throws AnalysisException { throw new AnalysisException("index definition expected."); } indexDef.analyze(); + indexDef.setColumnUniqueIds(getColumnUniqueIds()); this.index = new Index(Env.getCurrentEnv().getNextId(), indexDef.getIndexName(), indexDef.getColumns(), indexDef.getIndexType(), - indexDef.getProperties(), indexDef.getComment()); + indexDef.getProperties(), indexDef.getComment(), indexDef.getColumnUniqueIds()); + } + + private List getColumnUniqueIds() throws AnalysisException { + List ids = new ArrayList<>(); + String ctlName = super.tableName.getCtl(); + String dbName = super.tableName.getDb(); + String tblName = super.tableName.getTbl(); + DatabaseIf dbIf; + TableIf tableIf; + try { + dbIf = Env.getCurrentEnv().getCatalogMgr() + .getCatalogOrException(ctlName, catalog -> new DdlException("Unknown catalog " + catalog)) + .getDbOrDdlException(dbName); + tableIf = dbIf.getTableOrDdlException(tblName); + } catch (DdlException e) { + throw new AnalysisException("Failed to get table: " + e.getMessage()); + } + if (tableIf.getType() == TableIf.TableType.OLAP) { + OlapTable table = (OlapTable) tableIf; + for (Index index : table.getIndexes()) { + ids.addAll(index.getColumnUniqueIds()); + } + } + + return ids; } @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 index b39c0df4a85db5a..2e35f42a9a42da0 100644 --- 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 @@ -18,12 +18,18 @@ package org.apache.doris.analysis; import org.apache.doris.alter.AlterOpType; +import org.apache.doris.catalog.DatabaseIf; import org.apache.doris.catalog.Env; import org.apache.doris.catalog.Index; +import org.apache.doris.catalog.OlapTable; +import org.apache.doris.catalog.TableIf; import org.apache.doris.common.AnalysisException; +import org.apache.doris.common.DdlException; import com.google.common.collect.Maps; +import java.util.ArrayList; +import java.util.List; import java.util.Map; public class CreateIndexClause extends AlterTableClause { @@ -71,9 +77,35 @@ public void analyze(Analyzer analyzer) throws AnalysisException { throw new AnalysisException("index definition expected."); } indexDef.analyze(); + indexDef.setColumnUniqueIds(getColumnUniqueIds()); this.index = new Index(Env.getCurrentEnv().getNextId(), indexDef.getIndexName(), indexDef.getColumns(), indexDef.getIndexType(), - indexDef.getProperties(), indexDef.getComment()); + indexDef.getProperties(), indexDef.getComment(), indexDef.getColumnUniqueIds()); + } + + private List getColumnUniqueIds() throws AnalysisException { + List ids = new ArrayList<>(); + String ctlName = super.tableName.getCtl(); + String dbName = super.tableName.getDb(); + String tblName = super.tableName.getTbl(); + DatabaseIf dbIf; + TableIf tableIf; + try { + dbIf = Env.getCurrentEnv().getCatalogMgr() + .getCatalogOrException(ctlName, catalog -> new DdlException("Unknown catalog " + catalog)) + .getDbOrDdlException(dbName); + tableIf = dbIf.getTableOrDdlException(tblName); + } catch (DdlException e) { + throw new AnalysisException("Failed to get table: " + e.getMessage()); + } + if (tableIf.getType() == TableIf.TableType.OLAP) { + OlapTable table = (OlapTable) tableIf; + for (String column : indexDef.getColumns()) { + ids.add(table.getColumn(column).getUniqueId()); + } + } + + return ids; } @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 index f92da90e5c43bd3..b07424056906b87 100644 --- 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 @@ -618,7 +618,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 index 87bf7c5aa189cbd..4d3ae2f85915935 100644 --- 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 properties; private boolean isBuildDeferred = false; private PartitionNames partitionNames; + private List 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,14 @@ public List getPartitionNames() { return partitionNames == null ? Lists.newArrayList() : partitionNames.getPartitionNames(); } + public List getColumnUniqueIds() { + return columnUniqueIds; + } + + public void setColumnUniqueIds(List columnUniqueIds) { + this.columnUniqueIds = columnUniqueIds; + } + public enum IndexType { BITMAP, INVERTED, @@ -209,6 +218,7 @@ public boolean isInvertedIndex() { public void checkColumn(Column column, KeysType keysType, boolean enableUniqueKeyMergeOnWrite) throws AnalysisException { + columnUniqueIds.add(column.getUniqueId()); if (indexType == IndexType.BITMAP || indexType == IndexType.INVERTED || indexType == IndexType.BLOOMFILTER || indexType == IndexType.NGRAM_BF) { String indexColName = column.getName(); 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 index 56a878c8f93948a..40db2f1d5b01d50 100644 --- 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 properties; @SerializedName(value = "ct", alternate = {"comment"}) private String comment; + @SerializedName(value = "cui", alternate = {"columnUniqueIds"}) + private List columnUniqueIds; public Index(long indexId, String indexName, List columns, - IndexDef.IndexType indexType, Map properties, String comment) { + IndexDef.IndexType indexType, Map properties, String comment, + List 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 getColumnUniqueIds() { + return columnUniqueIds; + } + + public void setColumnUniqueIds(List 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/nereids/trees/plans/commands/info/IndexDefinition.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/IndexDefinition.java index d134687106e2568..031d4203b2f4859 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/IndexDefinition.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/IndexDefinition.java @@ -20,6 +20,7 @@ import org.apache.doris.analysis.IndexDef; import org.apache.doris.analysis.IndexDef.IndexType; import org.apache.doris.analysis.InvertedIndexUtil; +import org.apache.doris.catalog.Column; import org.apache.doris.catalog.Env; import org.apache.doris.catalog.Index; import org.apache.doris.catalog.KeysType; @@ -49,6 +50,7 @@ public class IndexDefinition { private IndexType indexType; private Map properties = new HashMap<>(); private boolean isBuildDeferred = false; + private List columnUniqueIds = Lists.newArrayList(); /** * constructor for IndexDefinition @@ -149,6 +151,8 @@ public void checkColumn(ColumnDefinition column, KeysType keysType, throw new AnalysisException("invalid ngram properties:" + e.getMessage(), e); } } + // fill columnUniqueIds + columnUniqueIds.add(Column.COLUMN_UNIQUE_ID_INIT_VALUE); } else { throw new AnalysisException("Unsupported index type: " + indexType); } @@ -207,6 +211,6 @@ public IndexType getIndexType() { public Index translateToCatalogStyle() { return new Index(Env.getCurrentEnv().getNextId(), name, cols, indexType, properties, - comment); + comment, columnUniqueIds); } } 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 index 950371de3034bd7..84b8c6062d2351b 100644 --- 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 index aeb5bc471fee421..966f6c38b5b7833 100644 --- 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 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 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 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 index 0e04bddd681fddf..be71998eac3f778 100644 --- 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 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 index 7d74c1773fec88e..d48d38343762ab9 100644 --- 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 properties + 7: optional list column_unique_ids } struct TOlapTableIndexSchema { diff --git a/regression-test/suites/inverted_index_p0/index_change/test_index_change_on_renamed_column.groovy b/regression-test/suites/inverted_index_p0/index_change/test_index_change_on_renamed_column.groovy index 3913b60786ed48a..6c8b8c4e0d8fbc7 100644 --- a/regression-test/suites/inverted_index_p0/index_change/test_index_change_on_renamed_column.groovy +++ b/regression-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)