This is an automated email from the ASF dual-hosted git repository. airborne pushed a commit to branch branch-3.0 in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-3.0 by this push: new 5ced8baa4ea [fix](bf index) add ngram bf index validation in nereids index definition check (#45780) (#46215) 5ced8baa4ea is described below commit 5ced8baa4ea75192eeeaac773e90f84dbd7f392e Author: airborne12 <jiang...@selectdb.com> AuthorDate: Wed Jan 1 18:09:26 2025 +0800 [fix](bf index) add ngram bf index validation in nereids index definition check (#45780) (#46215) pick #45780 --- .../java/org/apache/doris/analysis/IndexDef.java | 10 ++-- .../trees/plans/commands/info/IndexDefinition.java | 18 ++---- .../trees/plans/commands/IndexDefinitionTest.java | 64 +++++++++++++++++++++- .../index_p0/test_ngram_bloomfilter_index.groovy | 6 +- 4 files changed, 77 insertions(+), 21 deletions(-) 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 f51e63e4fbe..3e44d7c76ad 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 @@ -43,10 +43,10 @@ public class IndexDef { private boolean isBuildDeferred = false; private PartitionNames partitionNames; private List<Integer> columnUniqueIds = Lists.newArrayList(); - private static final int MIN_NGRAM_SIZE = 1; - private static final int MAX_NGRAM_SIZE = 255; - private static final int MIN_BF_SIZE = 64; - private static final int MAX_BF_SIZE = 65535; + public static final int MIN_NGRAM_SIZE = 1; + public static final int MAX_NGRAM_SIZE = 255; + public static final int MIN_BF_SIZE = 64; + public static final int MAX_BF_SIZE = 65535; public static final String NGRAM_SIZE_KEY = "gram_size"; public static final String NGRAM_BF_SIZE_KEY = "bf_size"; @@ -270,7 +270,7 @@ public class IndexDef { } } - private void parseAndValidateProperty(Map<String, String> properties, String key, int minValue, int maxValue) + public static void parseAndValidateProperty(Map<String, String> properties, String key, int minValue, int maxValue) throws AnalysisException { String valueStr = properties.get(key); if (valueStr == null) { 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 4d22e5af51c..9ee52e5b7a5 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 @@ -143,18 +143,12 @@ public class IndexDefinition { "ngram_bf index should have gram_size and bf_size properties"); } try { - int ngramSize = Integer.parseInt(properties.get(IndexDef.NGRAM_SIZE_KEY)); - int bfSize = Integer.parseInt(properties.get(IndexDef.NGRAM_BF_SIZE_KEY)); - if (ngramSize > 256 || ngramSize < 1) { - throw new AnalysisException( - "gram_size should be integer and less than 256"); - } - if (bfSize > 65535 || bfSize < 64) { - throw new AnalysisException( - "bf_size should be integer and between 64 and 65535"); - } - } catch (NumberFormatException e) { - throw new AnalysisException("invalid ngram properties:" + e.getMessage(), e); + IndexDef.parseAndValidateProperty(properties, IndexDef.NGRAM_SIZE_KEY, IndexDef.MIN_NGRAM_SIZE, + IndexDef.MAX_NGRAM_SIZE); + IndexDef.parseAndValidateProperty(properties, IndexDef.NGRAM_BF_SIZE_KEY, IndexDef.MIN_BF_SIZE, + IndexDef.MAX_BF_SIZE); + } catch (Exception ex) { + throw new AnalysisException("invalid ngram bf index params:" + ex.getMessage(), ex); } } } else { diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/IndexDefinitionTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/IndexDefinitionTest.java index 0632945ee04..2afff52c40e 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/IndexDefinitionTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/IndexDefinitionTest.java @@ -22,12 +22,17 @@ import org.apache.doris.catalog.KeysType; import org.apache.doris.nereids.exceptions.AnalysisException; import org.apache.doris.nereids.trees.plans.commands.info.ColumnDefinition; import org.apache.doris.nereids.trees.plans.commands.info.IndexDefinition; +import org.apache.doris.nereids.types.IntegerType; +import org.apache.doris.nereids.types.StringType; import org.apache.doris.nereids.types.VariantType; import com.google.common.collect.Lists; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import java.util.HashMap; +import java.util.Map; + public class IndexDefinitionTest { @Test void testVariantIndexFormatV1() throws AnalysisException { @@ -39,8 +44,65 @@ public class IndexDefinitionTest { null, "comment"), KeysType.UNIQUE_KEYS, true, isIndexFormatV1); Assertions.fail("No exception throws."); } catch (AnalysisException e) { - Assertions.assertTrue(e instanceof AnalysisException); + org.junit.jupiter.api.Assertions.assertInstanceOf( + org.apache.doris.nereids.exceptions.AnalysisException.class, e); Assertions.assertTrue(e.getMessage().contains("not supported in inverted index format V1")); } } + + @Test + void testNgramBFIndex() throws AnalysisException { + Map<String, String> properties = new HashMap<>(); + properties.put("gram_size", "3"); + properties.put("bf_size", "10000"); + + IndexDefinition def = new IndexDefinition("ngram_bf_index", Lists.newArrayList("col1"), "NGRAM_BF", + properties, "comment"); + def.checkColumn( + new ColumnDefinition("col1", StringType.INSTANCE, false, AggregateType.NONE, true, null, "comment"), + KeysType.DUP_KEYS, false, false); + } + + @Test + void testInvalidNgramBFIndexColumnType() { + Map<String, String> properties = new HashMap<>(); + properties.put("gram_size", "3"); + properties.put("bf_size", "10000"); + + IndexDefinition def = new IndexDefinition("ngram_bf_index", Lists.newArrayList("col1"), "NGRAM_BF", + properties, "comment"); + Assertions.assertThrows(AnalysisException.class, () -> + def.checkColumn( + new ColumnDefinition("col1", IntegerType.INSTANCE, false, AggregateType.NONE, true, null, + "comment"), + KeysType.DUP_KEYS, false, false)); + } + + @Test + void testNgramBFIndexInvalidSize() { + Map<String, String> properties = new HashMap<>(); + properties.put("gram_size", "256"); + properties.put("bf_size", "10000"); + + IndexDefinition def = new IndexDefinition("ngram_bf_index", Lists.newArrayList("col1"), "NGRAM_BF", + properties, "comment"); + Assertions.assertThrows(AnalysisException.class, () -> + def.checkColumn(new ColumnDefinition("col1", StringType.INSTANCE, false, AggregateType.NONE, true, null, + "comment"), + KeysType.DUP_KEYS, false, false)); + } + + @Test + void testNgramBFIndexInvalidSize2() { + Map<String, String> properties = new HashMap<>(); + properties.put("gram_size", "3"); + properties.put("bf_size", "65536"); + + IndexDefinition def = new IndexDefinition("ngram_bf_index", Lists.newArrayList("col1"), "NGRAM_BF", + properties, "comment"); + Assertions.assertThrows(AnalysisException.class, () -> + def.checkColumn(new ColumnDefinition("col1", StringType.INSTANCE, false, AggregateType.NONE, true, null, + "comment"), + KeysType.DUP_KEYS, false, false)); + } } diff --git a/regression-test/suites/index_p0/test_ngram_bloomfilter_index.groovy b/regression-test/suites/index_p0/test_ngram_bloomfilter_index.groovy index cce6ed9fd9d..04148e984cd 100644 --- a/regression-test/suites/index_p0/test_ngram_bloomfilter_index.groovy +++ b/regression-test/suites/index_p0/test_ngram_bloomfilter_index.groovy @@ -81,7 +81,7 @@ suite("test_ngram_bloomfilter_index") { DISTRIBUTED BY HASH(`key_id`) BUCKETS 3 PROPERTIES("replication_num" = "1"); """ - exception "bf_size should be integer and between 64 and 65535" + exception "java.sql.SQLException: errCode = 2, detailMessage = invalid ngram bf index params:errCode = 2, detailMessage = 'bf_size' should be an integer between 64 and 65535." } def tableName3 = 'test_ngram_bloomfilter_index3' @@ -104,10 +104,10 @@ suite("test_ngram_bloomfilter_index") { """ test { sql """ALTER TABLE ${tableName3} ADD INDEX idx_http_url(http_url) USING NGRAM_BF PROPERTIES("gram_size"="3", "bf_size"="65536") COMMENT 'http_url ngram_bf index'""" - exception "'bf_size' should be an integer between 64 and 65535" + exception "java.sql.SQLException: errCode = 2, detailMessage = 'bf_size' should be an integer between 64 and 65535." } test { sql """ALTER TABLE ${tableName3} ADD INDEX idx_http_url(http_url) USING NGRAM_BF PROPERTIES("gram_size"="256", "bf_size"="65535") COMMENT 'http_url ngram_bf index'""" - exception "'gram_size' should be an integer between 1 and 255" + exception "java.sql.SQLException: errCode = 2, detailMessage = 'gram_size' should be an integer between 1 and 255." } } --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org