From b14cd87a133700eaeadc0a17dc78c69c1e1eb82e Mon Sep 17 00:00:00 2001 From: qidaye Date: Wed, 20 Nov 2024 21:56:18 +0800 Subject: [PATCH 1/2] [fix](bloom filter)Fix drop column with bloom filter 1. When drop column with bloom filter, we can not do light schema change. 2. Set light_schema_change to false, to log the operation in editLog. --- .../doris/alter/SchemaChangeHandler.java | 9 ++++-- .../test_bloom_filter_drop_column.out | 2 +- .../test_bloom_filter_drop_column.groovy | 28 ++++++++++++++----- 3 files changed, 28 insertions(+), 11 deletions(-) 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 b7a0fa5cfbc746..d10bbbf8889e7d 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 @@ -292,7 +292,8 @@ private void processDropColumn(DropColumnClause alterClause, Table externalTable * @throws DdlException */ private boolean processDropColumn(DropColumnClause alterClause, OlapTable olapTable, - Map> indexSchemaMap, List indexes) + Map> indexSchemaMap, List indexes, + Map propertyMap) throws DdlException { String dropColName = alterClause.getColName(); @@ -440,7 +441,9 @@ private boolean processDropColumn(DropColumnClause alterClause, OlapTable olapTa newBfCols.add(bfCol); } } - olapTable.setBloomFilterInfo(newBfCols, olapTable.getBfFpp()); + propertyMap.put(PropertyAnalyzer.PROPERTIES_BF_COLUMNS, Joiner.on(",").join(newBfCols)); + // drop bloom filter column, should write editlog and can not do light schema change + lightSchemaChange = false; } for (int i = 1; i < indexIds.size(); i++) { @@ -2077,7 +2080,7 @@ public int getAsInt() { } else if (alterClause instanceof DropColumnClause) { // drop column and drop indexes on this column boolean clauseCanLigthSchemaChange = processDropColumn((DropColumnClause) alterClause, olapTable, - indexSchemaMap, newIndexes); + indexSchemaMap, newIndexes, propertyMap); if (!clauseCanLigthSchemaChange) { lightSchemaChange = false; } diff --git a/regression-test/data/bloom_filter_p0/test_bloom_filter_drop_column.out b/regression-test/data/bloom_filter_p0/test_bloom_filter_drop_column.out index 2c6ca8d224b728..14334dfb4b5c48 100644 --- a/regression-test/data/bloom_filter_p0/test_bloom_filter_drop_column.out +++ b/regression-test/data/bloom_filter_p0/test_bloom_filter_drop_column.out @@ -1,6 +1,6 @@ -- This file is automatically generated. You should know what you did if you want to edit this -- !select -- -1 1 +1 1 1 -- !select -- 1 \N diff --git a/regression-test/suites/bloom_filter_p0/test_bloom_filter_drop_column.groovy b/regression-test/suites/bloom_filter_p0/test_bloom_filter_drop_column.groovy index f426d4fca10a79..d83c70af30c709 100644 --- a/regression-test/suites/bloom_filter_p0/test_bloom_filter_drop_column.groovy +++ b/regression-test/suites/bloom_filter_p0/test_bloom_filter_drop_column.groovy @@ -21,13 +21,14 @@ suite("test_bloom_filter_drop_column") { sql """CREATE TABLE IF NOT EXISTS ${table_name} ( `a` varchar(150) NULL, - `c1` varchar(10) + `c1` varchar(10), + `c2` varchar(10) ) ENGINE=OLAP DUPLICATE KEY(`a`) DISTRIBUTED BY HASH(`a`) BUCKETS 1 PROPERTIES ( "replication_allocation" = "tag.location.default: 1", - "bloom_filter_columns" = "c1", + "bloom_filter_columns" = "c1, c2", "in_memory" = "false", "storage_format" = "V2" )""" @@ -51,12 +52,12 @@ suite("test_bloom_filter_drop_column") { assertTrue(useTime <= OpTimeout, "wait_for_latest_op_on_table_finish timeout") } - def assertShowCreateTableWithRetry = { tableName, expectedCondition, maxRetries, waitSeconds -> + def assertShowCreateTableWithRetry = { tableName, expectedCondition, contains, maxRetries, waitSeconds -> int attempt = 0 while (attempt < maxRetries) { def res = sql """SHOW CREATE TABLE ${tableName}""" log.info("Attempt ${attempt + 1}: show table: ${res}") - if (res && res.size() > 0 && res[0][1].contains(expectedCondition)) { + if (res && res.size() > 0 && ((contains && res[0][1].contains(expectedCondition)) || (!contains && !res[0][1].contains(expectedCondition)))) { logger.info("Attempt ${attempt + 1}: Condition met.") return } else { @@ -70,21 +71,34 @@ suite("test_bloom_filter_drop_column") { def finalRes = sql """SHOW CREATE TABLE ${tableName}""" log.info("Final attempt: show table: ${finalRes}") assertTrue(finalRes && finalRes.size() > 0, "SHOW CREATE TABLE return empty or null") - assertTrue(finalRes[0][1].contains(expectedCondition), "expected\"${expectedCondition}\"，actural: ${finalRes[0][1]}") + if (contains) { + assertTrue(finalRes[0][1].contains(expectedCondition), "expected to contain \"${expectedCondition}\", actual: ${finalRes[0][1]}") + } else { + assertTrue(!finalRes[0][1].contains(expectedCondition), "expected not to contain \"${expectedCondition}\", actual: ${finalRes[0][1]}") + } } - sql """INSERT INTO ${table_name} values ('1', '1')""" + sql """INSERT INTO ${table_name} values ('1', '1', '1')""" sql "sync" qt_select """select * from ${table_name} order by a""" + assertShowCreateTableWithRetry(table_name, "\"bloom_filter_columns\" = \"c1, c2\"", true, 3, 30) // drop column c1 sql """ALTER TABLE ${table_name} DROP COLUMN c1""" wait_for_latest_op_on_table_finish(table_name, timeout) sql "sync" // show create table with retry logic - assertShowCreateTableWithRetry(table_name, "\"bloom_filter_columns\" = \"\"", 3, 30) + assertShowCreateTableWithRetry(table_name, "\"bloom_filter_columns\" = \"c2\"", true, 3, 30) + + // drop column c2 + sql """ALTER TABLE ${table_name} DROP COLUMN c2""" + wait_for_latest_op_on_table_finish(table_name, timeout) + sql "sync" + + // show create table with retry logic + assertShowCreateTableWithRetry(table_name, "\"bloom_filter_columns\" = \"\"", false, 3, 30) // add new column c1 sql """ALTER TABLE ${table_name} ADD COLUMN c1 ARRAY""" From 9f063ffe153f96436faa24cdbcdf3389146ebc60 Mon Sep 17 00:00:00 2001 From: qidaye Date: Thu, 21 Nov 2024 16:36:21 +0800 Subject: [PATCH 2/2] rebuild bf info when replay editlog --- .../doris/alter/SchemaChangeHandler.java | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 deletions(-) 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 d10bbbf8889e7d..6eaf7d5522c96d 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 @@ -292,8 +292,7 @@ private void processDropColumn(DropColumnClause alterClause, Table externalTable * @throws DdlException */ private boolean processDropColumn(DropColumnClause alterClause, OlapTable olapTable, - Map> indexSchemaMap, List indexes, - Map propertyMap) + Map> indexSchemaMap, List indexes) throws DdlException { String dropColName = alterClause.getColName(); @@ -435,15 +434,16 @@ private boolean processDropColumn(DropColumnClause alterClause, OlapTable olapTa // drop bloom filter column Set bfCols = olapTable.getCopiedBfColumns(); if (bfCols != null) { - Set newBfCols = new HashSet<>(); + Set newBfCols = null; for (String bfCol : bfCols) { if (!bfCol.equalsIgnoreCase(dropColName)) { + if (newBfCols == null) { + newBfCols = Sets.newHashSet(); + } newBfCols.add(bfCol); } } - propertyMap.put(PropertyAnalyzer.PROPERTIES_BF_COLUMNS, Joiner.on(",").join(newBfCols)); - // drop bloom filter column, should write editlog and can not do light schema change - lightSchemaChange = false; + olapTable.setBloomFilterInfo(newBfCols, olapTable.getBfFpp()); } for (int i = 1; i < indexIds.size(); i++) { @@ -2080,7 +2080,7 @@ public int getAsInt() { } else if (alterClause instanceof DropColumnClause) { // drop column and drop indexes on this column boolean clauseCanLigthSchemaChange = processDropColumn((DropColumnClause) alterClause, olapTable, - indexSchemaMap, newIndexes, propertyMap); + indexSchemaMap, newIndexes); if (!clauseCanLigthSchemaChange) { lightSchemaChange = false; } @@ -2956,6 +2956,25 @@ public void modifyTableLightSchemaChange(String rawSql, Database db, OlapTable o LOG.info("finished modify table's add or drop or modify columns. table: {}, job: {}, is replay: {}", olapTable.getName(), jobId, isReplay); } + // for bloom filter, rebuild bloom filter info by table schema in replay + if (isReplay) { + Set bfCols = olapTable.getCopiedBfColumns(); + if (bfCols != null) { + List columns = olapTable.getBaseSchema(); + Set newBfCols = null; + for (String bfCol : bfCols) { + for (Column column : columns) { + if (column.getName().equalsIgnoreCase(bfCol)) { + if (newBfCols == null) { + newBfCols = Sets.newHashSet(); + } + newBfCols.add(column.getName()); + } + } + } + olapTable.setBloomFilterInfo(newBfCols, olapTable.getBfFpp()); + } + } } public void replayModifyTableLightSchemaChange(TableAddOrDropColumnsInfo info)