From ebfae38c452419f98d169baf5dde22865091337f Mon Sep 17 00:00:00 2001 From: "shuming.li" Date: Sat, 4 Jan 2025 10:36:35 +0800 Subject: [PATCH] fix bugs Signed-off-by: shuming.li --- .../starrocks/sql/analyzer/AnalyzerUtils.java | 24 +++++- .../sql/analyzer/AnalyzeTestUtil.java | 17 +++- .../sql/analyzer/AnalyzeUtilTest.java | 79 +++++++++++++++++++ .../test_automatic_partition_with_case_names | 34 +++++++- .../test_automatic_partition_with_case_names | 6 ++ 5 files changed, 152 insertions(+), 8 deletions(-) diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/AnalyzerUtils.java b/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/AnalyzerUtils.java index 1aa6f5ae8547b7..294fc3262e3fff 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/AnalyzerUtils.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/AnalyzerUtils.java @@ -1421,12 +1421,10 @@ public static AddPartitionClause getAddPartitionClauseFromPartitionValues(OlapTa if (!partitionColNames.contains(partitionName)) { List> partitionItems = Collections.singletonList(partitionValue); PListCell cell = new PListCell(partitionItems); - // If partition name already exists and their partition values are not the same, change partition name. + // If partition name already exists and their partition values are different, change partition name. if (tablePartitions.containsKey(partitionName) && !tablePartitions.get(partitionName).equals(cell)) { // change partition name, how to generate a unique partition name - int diff = calculateStringDiff(partitionName, partitionName.toLowerCase(Locale.ROOT)); - partitionName = partitionName + "_" + Integer.toHexString(diff); - // ensure partition name is unique with case-insensitive + partitionName = calculateUniquePartitionName(partitionName, tablePartitions); if (tablePartitions.containsKey(partitionName)) { throw new AnalysisException("partition name " + partitionName + " already exists."); } @@ -1453,6 +1451,24 @@ public static AddPartitionClause getAddPartitionClauseFromPartitionValues(OlapTa /** * Calculate the difference between two strings which have the same length. */ + private static String calculateUniquePartitionName(String partitionName, + Map tablePartitions) { + // change partition name, how to generate a unique partition name + String newPartitionName = partitionName + "_" + Integer.toHexString(partitionName.hashCode()); + // ensure partition name is unique with case-insensitive + if (tablePartitions.containsKey(newPartitionName)) { + int diff = calculateStringDiff(partitionName, partitionName.toUpperCase(Locale.ROOT)); + int i = 0; + do { + newPartitionName = partitionName + "_" + Integer.toHexString(diff + (i++)); + if (i > 100) { + break; + } + } while (tablePartitions.containsKey(newPartitionName)); + } + return newPartitionName; + } + private static int calculateStringDiff(String str1, String str2) { if (str1 == null || str2 == null) { return 0; diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/analyzer/AnalyzeTestUtil.java b/fe/fe-core/src/test/java/com/starrocks/sql/analyzer/AnalyzeTestUtil.java index 2ba43bac4fc0b7..0a79e254d29f24 100644 --- a/fe/fe-core/src/test/java/com/starrocks/sql/analyzer/AnalyzeTestUtil.java +++ b/fe/fe-core/src/test/java/com/starrocks/sql/analyzer/AnalyzeTestUtil.java @@ -28,9 +28,9 @@ import org.junit.Assert; public class AnalyzeTestUtil { - private static ConnectContext connectContext; - private static StarRocksAssert starRocksAssert; - private static String DB_NAME = "test"; + protected static ConnectContext connectContext; + protected static StarRocksAssert starRocksAssert; + protected static String DB_NAME = "test"; public static void init() throws Exception { Config.enable_experimental_rowstore = true; @@ -315,6 +315,17 @@ public static void init() throws Exception { "\"replication_num\" = \"1\",\n" + "\"storage_type\" = \"column_with_row\"" + ");"); + starRocksAssert.withTable("CREATE TABLE test.auto_tbl1 (\n" + + " col1 varchar(100),\n" + + " col2 varchar(100),\n" + + " col3 bigint\n" + + ") ENGINE=OLAP\n" + + "PRIMARY KEY (col1)\n" + + "PARTITION BY (col1)\n" + + "DISTRIBUTED BY HASH(col1) BUCKETS 5\n" + + "PROPERTIES (\n" + + "\"replication_num\" = \"1\"\n" + + ");"); } public static String getDbName() { diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/analyzer/AnalyzeUtilTest.java b/fe/fe-core/src/test/java/com/starrocks/sql/analyzer/AnalyzeUtilTest.java index 4683c58b0e87c4..df01e03a0bd063 100644 --- a/fe/fe-core/src/test/java/com/starrocks/sql/analyzer/AnalyzeUtilTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/sql/analyzer/AnalyzeUtilTest.java @@ -16,13 +16,19 @@ package com.starrocks.sql.analyzer; import com.google.common.base.Strings; +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; import com.starrocks.analysis.TableName; import com.starrocks.catalog.Database; +import com.starrocks.catalog.OlapTable; import com.starrocks.catalog.Table; import com.starrocks.common.Pair; import com.starrocks.qe.ConnectContext; import com.starrocks.server.GlobalStateMgr; +import com.starrocks.sql.ast.AddPartitionClause; import com.starrocks.sql.ast.CreateViewStmt; +import com.starrocks.sql.ast.ListPartitionDesc; +import com.starrocks.sql.ast.PartitionDesc; import com.starrocks.sql.ast.StatementBase; import com.starrocks.sql.parser.SqlParser; import com.starrocks.utframe.UtFrameUtils; @@ -33,8 +39,11 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; +import static com.starrocks.sql.analyzer.AnalyzeTestUtil.DB_NAME; import static com.starrocks.sql.analyzer.AnalyzeTestUtil.analyzeSuccess; +import static com.starrocks.sql.analyzer.AnalyzeTestUtil.starRocksAssert; public class AnalyzeUtilTest { @BeforeClass @@ -408,4 +417,74 @@ public void testContainsNonDeterministicFunction() { Assert.assertTrue(!Strings.isNullOrEmpty(result.second)); } } + @Test + public void testCalculateStringDiff() throws Exception { + OlapTable t1 = (OlapTable) starRocksAssert.getTable(DB_NAME, "auto_tbl1"); + List combinations = generateCaseCombinations("abc_def?ghi|g.com"); + List> partitionValues = combinations.stream() + .map(s -> Lists.newArrayList(s)) + .collect(Collectors.toList()); + Map partitionNames = Maps.newTreeMap(String.CASE_INSENSITIVE_ORDER); + { + AddPartitionClause addPartitionClauses = + AnalyzerUtils.getAddPartitionClauseFromPartitionValues(t1, partitionValues, false, ""); + ListPartitionDesc listPartitionDesc = (ListPartitionDesc) addPartitionClauses.getPartitionDesc(); + List descs = listPartitionDesc.getPartitionDescs(); + Assert.assertEquals(combinations.size(), descs.size()); + for (PartitionDesc desc : descs) { + Assert.assertTrue(partitionNames.get(desc.getPartitionName()) == null); + partitionNames.put(desc.getPartitionName(), desc); + } + } + + { + AddPartitionClause addPartitionClauses = + AnalyzerUtils.getAddPartitionClauseFromPartitionValues(t1, partitionValues, false, ""); + ListPartitionDesc listPartitionDesc = (ListPartitionDesc) addPartitionClauses.getPartitionDesc(); + List descs = listPartitionDesc.getPartitionDescs(); + Assert.assertEquals(combinations.size(), descs.size()); + for (PartitionDesc desc : descs) { + Assert.assertTrue(partitionNames.get(desc.getPartitionName()) != null); + } + } + + { + AddPartitionClause addPartitionClauses = + AnalyzerUtils.getAddPartitionClauseFromPartitionValues(t1, partitionValues, false, ""); + ListPartitionDesc listPartitionDesc = (ListPartitionDesc) addPartitionClauses.getPartitionDesc(); + List descs = listPartitionDesc.getPartitionDescs(); + Assert.assertEquals(combinations.size(), descs.size()); + for (PartitionDesc desc : descs) { + Assert.assertTrue(partitionNames.get(desc.getPartitionName()) != null); + } + } + } + + public static List generateCaseCombinations(String input) { + List results = Lists.newArrayList(); + generateHelper(input.toCharArray(), 0, results); + return results; + } + + private static void generateHelper(char[] chars, int index, List results) { + if (index == chars.length) { + // Add the current combination to the results + results.add(new String(chars)); + return; + } + + // If the current character is alphabetic, generate both lowercase and uppercase + if (Character.isLetter(chars[index])) { + // Recurse with the lowercase version + chars[index] = Character.toLowerCase(chars[index]); + generateHelper(chars, index + 1, results); + + // Recurse with the uppercase version + chars[index] = Character.toUpperCase(chars[index]); + generateHelper(chars, index + 1, results); + } else { + // If not alphabetic, just continue to the next character + generateHelper(chars, index + 1, results); + } + } } diff --git a/test/sql/test_automatic_bucket/R/test_automatic_partition_with_case_names b/test/sql/test_automatic_bucket/R/test_automatic_partition_with_case_names index a63692f42dddb2..c64ba793c56b6b 100644 --- a/test/sql/test_automatic_bucket/R/test_automatic_partition_with_case_names +++ b/test/sql/test_automatic_bucket/R/test_automatic_partition_with_case_names @@ -19,15 +19,40 @@ ORDER BY (col2); insert into t1 values ('a.com', 'val1', 100), ('A.com', 'val1', 200), ('A.Com', 'val1', 300); -- result: -- !result +insert into t1 values ('a.com', 'val1', 100), ('A.com', 'val1', 200), ('A.Com', 'val1', 300); +-- result: +-- !result +insert into t1 values ('a.cOm', 'val1', 100), ('A.coM', 'val1', 200), ('A.COm', 'val1', 300); +-- result: +-- !result +insert into t1 values ('a.cOM', 'val1', 100), ('A.COM', 'val1', 200), ('a.COM', 'val1', 300); +-- result: +-- !result +insert into t1 values ('a.com', 'val1', 100), ('A.com', 'val1', 200), ('A.Com', 'val1', 300); +-- result: +-- !result +insert into t1 values ('a.cOm', 'val1', 100), ('A.coM', 'val1', 200), ('A.COm', 'val1', 300); +-- result: +-- !result +insert into t1 values ('b.cOm', 'val1', 100), ('A.coM', 'val1', 200), ('A.COm', 'val1', 300); +-- result: +-- !result SELECT count(1) FROM information_schema.partitions_meta WHERE DB_NAME='test_db_${uuid0}' AND table_name = 't1' ; -- result: -4 +11 -- !result select * from t1 order by col1, col2, col3; -- result: +A.COM val1 200 +A.COm val1 300 A.Com val1 300 +A.coM val1 200 A.com val1 200 +a.COM val1 300 +a.cOM val1 100 +a.cOm val1 100 a.com val1 100 +b.cOm val1 100 -- !result select * from t1 where col1 = 'a.com' order by col1, col2, col3; -- result: @@ -55,9 +80,16 @@ AS SELECT col1, sum(col3) from t1 group by col1; refresh materialized view test_async_mv with sync mode; select * from test_async_mv order by col1; -- result: +A.COM 200 +A.COm 300 A.Com 300 +A.coM 200 A.com 200 +a.COM 300 +a.cOM 100 +a.cOm 100 a.com 100 +b.cOm 100 -- !result select * from test_async_mv where col1 = 'a.com' order by col1; -- result: diff --git a/test/sql/test_automatic_bucket/T/test_automatic_partition_with_case_names b/test/sql/test_automatic_bucket/T/test_automatic_partition_with_case_names index 194dae3e3c3e69..9c0274472eb7a7 100644 --- a/test/sql/test_automatic_bucket/T/test_automatic_partition_with_case_names +++ b/test/sql/test_automatic_bucket/T/test_automatic_partition_with_case_names @@ -14,6 +14,12 @@ ORDER BY (col2); -- insert partition name with different case insert into t1 values ('a.com', 'val1', 100), ('A.com', 'val1', 200), ('A.Com', 'val1', 300); +insert into t1 values ('a.com', 'val1', 100), ('A.com', 'val1', 200), ('A.Com', 'val1', 300); +insert into t1 values ('a.cOm', 'val1', 100), ('A.coM', 'val1', 200), ('A.COm', 'val1', 300); +insert into t1 values ('a.cOM', 'val1', 100), ('A.COM', 'val1', 200), ('a.COM', 'val1', 300); +insert into t1 values ('a.com', 'val1', 100), ('A.com', 'val1', 200), ('A.Com', 'val1', 300); +insert into t1 values ('a.cOm', 'val1', 100), ('A.coM', 'val1', 200), ('A.COm', 'val1', 300); +insert into t1 values ('b.cOm', 'val1', 100), ('A.coM', 'val1', 200), ('A.COm', 'val1', 300); SELECT count(1) FROM information_schema.partitions_meta WHERE DB_NAME='test_db_${uuid0}' AND table_name = 't1' ;