[Bugfix] fix checkState error when predicate don't support dictionary…

… optimize (StarRocks#8700) when predicate don't support dictionary optimize, we will remove it from context.allstringcolumns. but when we join it with a table without any string columns. checkState will got an error
minchowang · Jul 14, 2022 · e7d28c0 · e7d28c0
1 parent e37db49
commit e7d28c0
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 9 deletions.
diff --git a/...ore/src/main/java/com/starrocks/sql/optimizer/rewrite/AddDecodeNodeForDictStringRule.java b/...ore/src/main/java/com/starrocks/sql/optimizer/rewrite/AddDecodeNodeForDictStringRule.java
@@ -356,7 +356,6 @@ public OptExpression visitPhysicalOlapScan(OptExpression optExpression, DecodeCo
                     };
 
                     if (!checkColumnCouldApply.getAsBoolean()) {
-                        context.allStringColumnIds.remove(columnId);
                         continue;
                     }
 
@@ -396,8 +395,6 @@ public OptExpression visitPhysicalOlapScan(OptExpression optExpression, DecodeCo
                     for (int i = 0; i < predicates.size(); i++) {
                         ScalarOperator predicate = predicates.get(i);
                         if (predicate.getUsedColumns().isIntersect(applyOptCols)) {
-                            Preconditions.checkState(
-                                    couldApplyDictOptimize(predicate, context.allStringColumnIds));
                             final DictMappingRewriter rewriter = new DictMappingRewriter(context);
                             final ScalarOperator newCallOperator = rewriter.rewrite(predicate.clone());
                             predicates.set(i, newCallOperator);

diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rewrite/DictMappingRewriter.java b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rewrite/DictMappingRewriter.java
@@ -33,9 +33,7 @@ public class DictMappingRewriter {
 
     public DictMappingRewriter(AddDecodeNodeForDictStringRule.DecodeContext decodeContext) {
         this.decodeContext = decodeContext;
-        for (Integer allStringColumnId : decodeContext.allStringColumnIds) {
-            rewriterContext.stringColumnSet.union(allStringColumnId);
-        }
+        decodeContext.stringColumnIdToDictColumnIds.keySet().forEach(rewriterContext.stringColumnSet::union);
     }
 
     public ScalarOperator rewrite(ScalarOperator scalarOperator) {
@@ -194,7 +192,7 @@ public ScalarOperator visitCaseWhenOperator(CaseWhenOperator operator, RewriterC
 
         @Override
         public ScalarOperator visitVariableReference(ColumnRefOperator operator, RewriterContext context) {
-            context.hasAppliedOperator = decodeContext.allStringColumnIds.contains(operator.getId());
+            context.hasAppliedOperator = rewriterContext.stringColumnSet.contains(operator.getId());
             context.hasUnsupportedOperator = false;
             return operator;
         }

diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/plan/LowCardinalityTest.java b/fe/fe-core/src/test/java/com/starrocks/sql/plan/LowCardinalityTest.java
@@ -33,6 +33,11 @@ public static void beforeClass() throws Exception {
                 "\"storage_format\" = \"DEFAULT\"\n" +
                 ");");
 
+        starRocksAssert.withTable("CREATE TABLE table_int (id_int INT, id_bigint BIGINT) " +
+                "DUPLICATE KEY(`id_int`) " +
+                "DISTRIBUTED BY HASH(`id_int`) BUCKETS 1 " +
+                "PROPERTIES (\"replication_num\" = \"1\");");
+
         starRocksAssert.withTable("CREATE TABLE part_v2  ( P_PARTKEY     INTEGER NOT NULL,\n" +
                 "                          P_NAME        VARCHAR(55) NOT NULL,\n" +
                 "                          P_MFGR        VARCHAR(25) NOT NULL,\n" +
@@ -678,7 +683,8 @@ public void testProject() throws Exception {
         // test worth for rewrite
         sql = "select concat(S_ADDRESS, S_COMMENT) from supplier";
         plan = getVerboseExplain(sql);
-        Assert.assertTrue(plan.contains("  |  9 <-> concat[([3: S_ADDRESS, VARCHAR, false], [7: S_COMMENT, VARCHAR, false]); args: VARCHAR; result: VARCHAR; args nullable: false; result nullable: true]"));
+        Assert.assertTrue(plan.contains(
+                "  |  9 <-> concat[([3: S_ADDRESS, VARCHAR, false], [7: S_COMMENT, VARCHAR, false]); args: VARCHAR; result: VARCHAR; args nullable: false; result nullable: true]"));
         // Test common expression reuse 1
         // couldn't reuse case
         // DictExpr return varchar and int
@@ -701,7 +707,8 @@ public void testProject() throws Exception {
         Assert.assertFalse(plan.contains("Decode"));
 
         // common expression reuse 3
-        sql = "select if(S_ADDRESS='kks', upper(S_COMMENT), S_COMMENT), concat(upper(S_COMMENT), S_ADDRESS) from supplier";
+        sql =
+                "select if(S_ADDRESS='kks', upper(S_COMMENT), S_COMMENT), concat(upper(S_COMMENT), S_ADDRESS) from supplier";
         plan = getVerboseExplain(sql);
         Assert.assertTrue(plan.contains("  |  output columns:\n" +
                 "  |  9 <-> if[(DictExpr(11: S_ADDRESS,[<place-holder> = 'kks']), [13: expr, VARCHAR, true], DictExpr(12: S_COMMENT,[<place-holder>])); args: BOOLEAN,VARCHAR,VARCHAR; result: VARCHAR; args nullable: true; result nullable: true]\n" +
@@ -820,6 +827,10 @@ public void testJoin() throws Exception {
         plan = getVerboseExplain(sql);
         Assert.assertFalse(plan.contains("Decode"));
 
+        sql =  "select count(*) from supplier l join [broadcast] (select max(id_int) as id_int from table_int) r on l.S_ADDRESS = r.id_int where l.S_ADDRESS not like '%key%'";
+        plan = getVerboseExplain(sql);
+        Assert.assertFalse(plan.contains("Decode"));
+
         sql = "select *\n" +
                 "from(\n" +
                 "        select S_SUPPKEY,\n" +