From 8d1553946a0db35f1aa4b30411be01b21b05bbe1 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 14 Jan 2025 19:32:47 +0800 Subject: [PATCH] branch-3.0: [fix](inverted index) Ensure that col_unique_ids in TabletIndex are not empty. #46648 (#46971) Cherry-picked from #46648 Co-authored-by: zzzxl --- be/src/olap/tablet_schema.h | 7 +++ .../java/org/apache/doris/catalog/Index.java | 11 ++-- .../datasource/CloudInternalCatalog.java | 7 ++- .../test_index_ddl_fault_injection.groovy | 57 +++++++++++++++---- 4 files changed, 63 insertions(+), 19 deletions(-) diff --git a/be/src/olap/tablet_schema.h b/be/src/olap/tablet_schema.h index 960b94ff910e49..d7e9c678ccc5e2 100644 --- a/be/src/olap/tablet_schema.h +++ b/be/src/olap/tablet_schema.h @@ -41,6 +41,7 @@ #include "runtime/define_primitive_type.h" #include "runtime/descriptors.h" #include "runtime/memory/lru_cache_policy.h" +#include "util/debug_points.h" #include "util/string_util.h" #include "vec/aggregate_functions/aggregate_function.h" #include "vec/common/string_ref.h" @@ -407,6 +408,12 @@ class TabletSchema : public MetadataAdder { } bool has_inverted_index() const { for (const auto& index : _indexes) { + DBUG_EXECUTE_IF("tablet_schema::has_inverted_index", { + if (index.col_unique_ids().empty()) { + throw Exception(Status::InternalError("col unique ids cannot be empty")); + } + }); + if (index.index_type() == IndexType::INVERTED) { //if index_id == -1, ignore it. if (!index.col_unique_ids().empty() && index.col_unique_ids()[0] >= 0) { 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 8d4cc0ee4aafb2..f770de9176f0db 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 @@ -264,15 +264,14 @@ public TOlapTableIndex toThrift() { return tIndex; } - public OlapFile.TabletIndexPB toPb(List schemaColumns) { + public OlapFile.TabletIndexPB toPb(Map columnMap) { OlapFile.TabletIndexPB.Builder builder = OlapFile.TabletIndexPB.newBuilder(); builder.setIndexId(indexId); builder.setIndexName(indexName); - for (String columnName : columns) { - for (Column column : schemaColumns) { - if (column.getName().equals(columnName)) { - builder.addColUniqueId(column.getUniqueId()); - } + for (Integer columnUniqueId : columnUniqueIds) { + Column column = columnMap.get(columnUniqueId); + if (column != null) { + builder.addColUniqueId(column.getUniqueId()); } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/cloud/datasource/CloudInternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/cloud/datasource/CloudInternalCatalog.java index 13485a02c4fd19..17bc301c050d83 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/cloud/datasource/CloudInternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/cloud/datasource/CloudInternalCatalog.java @@ -316,12 +316,17 @@ public OlapFile.TabletMetaCloudPB.Builder createTabletMetaBuilder(long tableId, schemaBuilder.addColumn(column.toPb(bfColumns, indexes)); } + Map columnMap = Maps.newHashMap(); + for (Column column : schemaColumns) { + columnMap.put(column.getUniqueId(), column); + } if (indexes != null) { for (int i = 0; i < indexes.size(); i++) { Index index = indexes.get(i); - schemaBuilder.addIndex(index.toPb(schemaColumns)); + schemaBuilder.addIndex(index.toPb(columnMap)); } } + if (rowStoreColumnUniqueIds != null) { schemaBuilder.addAllRowStoreColumnUniqueIds(rowStoreColumnUniqueIds); } diff --git a/regression-test/suites/fault_injection_p0/test_index_ddl_fault_injection.groovy b/regression-test/suites/fault_injection_p0/test_index_ddl_fault_injection.groovy index a7a025533a7765..4a907e8601ddae 100644 --- a/regression-test/suites/fault_injection_p0/test_index_ddl_fault_injection.groovy +++ b/regression-test/suites/fault_injection_p0/test_index_ddl_fault_injection.groovy @@ -18,10 +18,13 @@ import org.codehaus.groovy.runtime.IOGroovyMethods suite("test_index_ddl_fault_injection", "nonConcurrent") { + def tableName1 = "test_index_ddl_fault_injection_tbl_1" + def tableName2 = "test_index_ddl_fault_injection_tbl_2" + try { - sql "DROP TABLE IF EXISTS `test_index_ddl_fault_injection_tbl`" + sql "DROP TABLE IF EXISTS `${tableName1}`" sql """ - CREATE TABLE test_index_ddl_fault_injection_tbl ( + CREATE TABLE ${tableName1} ( `k1` int(11) NULL COMMENT "", `k2` int(11) NULL COMMENT "", `v1` string NULL COMMENT "" @@ -31,21 +34,21 @@ suite("test_index_ddl_fault_injection", "nonConcurrent") { PROPERTIES ( "replication_allocation" = "tag.location.default: 1"); """ - sql """ INSERT INTO test_index_ddl_fault_injection_tbl VALUES (1, 2, "hello"), (3, 4, "world"); """ + sql """ INSERT INTO ${tableName1} VALUES (1, 2, "hello"), (3, 4, "world"); """ sql 'sync' - qt_order1 """ select * from test_index_ddl_fault_injection_tbl where v1 = 'hello'; """ + qt_order1 """ select * from ${tableName1} where v1 = 'hello'; """ // add bloom filter - sql """ ALTER TABLE test_index_ddl_fault_injection_tbl set ("bloom_filter_columns" = "v1"); """ - assertEquals("FINISHED", getAlterColumnFinalState("test_index_ddl_fault_injection_tbl")) + sql """ ALTER TABLE ${tableName1} set ("bloom_filter_columns" = "v1"); """ + assertEquals("FINISHED", getAlterColumnFinalState("${tableName1}")) try { - qt_order2 """ select * from test_index_ddl_fault_injection_tbl where v1 = 'hello'; """ + qt_order2 """ select * from ${tableName1} where v1 = 'hello'; """ GetDebugPoint().enableDebugPointForAllBEs("BloomFilterIndexReader::new_iterator.fail"); test { // if BE add bloom filter correctly, this query will call BloomFilterIndexReader::new_iterator - sql """ select * from test_index_ddl_fault_injection_tbl where v1 = 'hello'; """ + sql """ select * from ${tableName1} where v1 = 'hello'; """ exception "new_iterator for bloom filter index failed" } } finally { @@ -53,17 +56,47 @@ suite("test_index_ddl_fault_injection", "nonConcurrent") { } // drop bloom filter - sql """ ALTER TABLE test_index_ddl_fault_injection_tbl set ("bloom_filter_columns" = ""); """ - assertEquals("FINISHED", getAlterColumnFinalState("test_index_ddl_fault_injection_tbl")) + sql """ ALTER TABLE ${tableName1} set ("bloom_filter_columns" = ""); """ + assertEquals("FINISHED", getAlterColumnFinalState("${tableName1}")) try { - qt_order3 """ select * from test_index_ddl_fault_injection_tbl where v1 = 'hello'; """ + qt_order3 """ select * from ${tableName1} where v1 = 'hello'; """ GetDebugPoint().enableDebugPointForAllBEs("BloomFilterIndexReader::new_iterator.fail"); // if BE drop bloom filter correctly, this query will not call BloomFilterIndexReader::new_iterator - qt_order4 """ select * from test_index_ddl_fault_injection_tbl where v1 = 'hello'; """ + qt_order4 """ select * from ${tableName1} where v1 = 'hello'; """ } finally { GetDebugPoint().disableDebugPointForAllBEs("BloomFilterIndexReader::new_iterator.fail"); } } finally { } + + sql "DROP TABLE IF EXISTS `${tableName2}`" + sql """ + CREATE TABLE `${tableName2}` ( + `col0` bigint NOT NULL, + `col1` boolean NULL, + `col2` tinyint NOT NULL, + INDEX col1 (`col1`) USING INVERTED, + INDEX col2 (`col2`) USING INVERTED + ) ENGINE=OLAP + UNIQUE KEY(`col0`) + DISTRIBUTED BY HASH(`col0`) BUCKETS 1 + PROPERTIES ( + "enable_unique_key_merge_on_write" = "true", + "store_row_column" = "true", + "replication_num" = "1" + ); + """ + + try { + sql """ INSERT INTO ${tableName2} (col0, col1, col2) VALUES (-74, true, 1); """ + sql 'sync' + + GetDebugPoint().enableDebugPointForAllBEs("tablet_schema::has_inverted_index"); + + sql """ ALTER TABLE ${tableName2} MODIFY COLUMN col1 BOOLEAN; """ + assertEquals("FINISHED", getAlterColumnFinalState("${tableName2}")) + } finally { + GetDebugPoint().disableDebugPointForAllBEs("tablet_schema::has_inverted_index"); + } }