From 75f3e38464c384b92483cd9290d02121f8e85352 Mon Sep 17 00:00:00 2001 From: silverbullet233 <3675229+silverbullet233@users.noreply.github.com> Date: Sun, 18 Dec 2022 21:48:10 +0800 Subject: [PATCH 1/4] fixup isNullable of cast --- .../src/main/java/com/starrocks/analysis/CastExpr.java | 4 ++++ .../sql/optimizer/operator/scalar/CastOperator.java | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/fe/fe-core/src/main/java/com/starrocks/analysis/CastExpr.java b/fe/fe-core/src/main/java/com/starrocks/analysis/CastExpr.java index deb00bfc81a89..3aae3510a47ce 100644 --- a/fe/fe-core/src/main/java/com/starrocks/analysis/CastExpr.java +++ b/fe/fe-core/src/main/java/com/starrocks/analysis/CastExpr.java @@ -229,6 +229,10 @@ public boolean canHashPartition() { @Override public boolean isNullable() { + Expr fromExpr = getChild(0); + if (ScalarType.isImplicitlyCastable(fromExpr.getType(), getType(), true)) { + return fromExpr.isNullable(); + } return true; } diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/operator/scalar/CastOperator.java b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/operator/scalar/CastOperator.java index 573ea85d49939..63f95cffbc24e 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/operator/scalar/CastOperator.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/operator/scalar/CastOperator.java @@ -16,6 +16,7 @@ package com.starrocks.sql.optimizer.operator.scalar; import com.google.common.collect.Lists; +import com.starrocks.catalog.ScalarType; import com.starrocks.catalog.Type; import java.util.Objects; @@ -43,6 +44,10 @@ public boolean isImplicit() { @Override public boolean isNullable() { + ScalarOperator fromOperator = getChild(0); + if (ScalarType.isImplicitlyCastable(fromOperator.getType(), getType(), true)) { + return fromOperator.isNullable(); + } return true; } From 8cc8ed4ad427cce510d400826e5ba5bba8781e8e Mon Sep 17 00:00:00 2001 From: silverbullet233 <3675229+silverbullet233@users.noreply.github.com> Date: Mon, 19 Dec 2022 14:44:29 +0800 Subject: [PATCH 2/4] fix ut --- ...lectStmtWithDecimalTypesNewPlannerTest.java | 18 +++++++++--------- .../plan/DistributedEnvPlanWithCostTest.java | 4 ++-- .../com/starrocks/sql/plan/PlanTestBase.java | 1 + .../java/com/starrocks/sql/plan/SetTest.java | 8 ++++---- .../resources/sql/tpch-histogram-cost/q11.sql | 10 ++++++++-- 5 files changed, 24 insertions(+), 17 deletions(-) diff --git a/fe/fe-core/src/test/java/com/starrocks/analysis/SelectStmtWithDecimalTypesNewPlannerTest.java b/fe/fe-core/src/test/java/com/starrocks/analysis/SelectStmtWithDecimalTypesNewPlannerTest.java index f2feb1c52d2d1..8b7ed179cca3d 100644 --- a/fe/fe-core/src/test/java/com/starrocks/analysis/SelectStmtWithDecimalTypesNewPlannerTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/analysis/SelectStmtWithDecimalTypesNewPlannerTest.java @@ -62,7 +62,7 @@ public static void setUp() throws Exception { starRocksAssert.withDatabase("db1").useDatabase("db1"); starRocksAssert.withTable(createTblStmtStr); starRocksAssert.withTable("CREATE TABLE `test_decimal_type6` (\n" + - " `dec_1_2` decimal32(2, 1) NOT NULL COMMENT \"\",\n" + + " `dec_2_1` decimal32(2, 1) NOT NULL COMMENT \"\",\n" + " `dec_18_0` decimal64(18, 0) NOT NULL COMMENT \"\",\n" + " `dec_18_18` decimal64(18, 18) NOT NULL COMMENT \"\",\n" + " `dec_10_2` decimal64(10, 2) NOT NULL COMMENT \"\",\n" + @@ -72,9 +72,9 @@ public static void setUp() throws Exception { " dec_4_2 decimal64(4, 2),\n" + " dec_5_1 decimal64(5, 1)\n" + ") ENGINE=OLAP\n" + - "DUPLICATE KEY(`dec_1_2`)\n" + + "DUPLICATE KEY(`dec_2_1`)\n" + "COMMENT \"OLAP\"\n" + - "DISTRIBUTED BY HASH(`dec_1_2`) BUCKETS 10\n" + + "DISTRIBUTED BY HASH(`dec_2_1`) BUCKETS 10\n" + "PROPERTIES (\n" + "\"replication_num\" = \"1\",\n" + "\"in_memory\" = \"false\",\n" + @@ -317,7 +317,7 @@ public void testDecimal32Stddev() throws Exception { String sql = "select stddev(col_decimal32p9s2) from db1.decimal_table"; String plan = UtFrameUtils.getVerboseFragmentPlan(ctx, sql); String snippet = "aggregate: stddev[(cast([2: col_decimal32p9s2, DECIMAL32(9,2), false] as DOUBLE)); " + - "args: DOUBLE; result: DOUBLE; args nullable: true; result nullable: true]"; + "args: DOUBLE; result: DOUBLE; args nullable: false; result nullable: true]"; Assert.assertTrue(plan.contains(snippet)); } @@ -326,7 +326,7 @@ public void testDecimal32Variance() throws Exception { String sql = "select variance(col_decimal32p9s2) from db1.decimal_table"; String plan = UtFrameUtils.getVerboseFragmentPlan(ctx, sql); String snippet = "aggregate: variance[(cast([2: col_decimal32p9s2, DECIMAL32(9,2), false] as DOUBLE)); " + - "args: DOUBLE; result: DOUBLE; args nullable: true; result nullable: true]"; + "args: DOUBLE; result: DOUBLE; args nullable: false; result nullable: true]"; Assert.assertTrue(plan.contains(snippet)); } @@ -402,7 +402,7 @@ public void testDecimalAddRule() throws Exception { // decimal(10, 2) + decimal(10, 2) = decimal(11, 2) sql = "select count(`dec_10_2` + dec_10_2) from `test_decimal_type6`;"; plan = UtFrameUtils.getVerboseFragmentPlan(ctx, sql); - Assert.assertTrue(plan.contains("[12: cast, DECIMAL64(11,2), true] + [12: cast, DECIMAL64(11,2), true]")); + Assert.assertTrue(plan.contains("[12: cast, DECIMAL64(11,2), false] + [12: cast, DECIMAL64(11,2), false]")); sql = "select count(`dec_10_2` + dec_12_10) from `test_decimal_type6`;"; plan = UtFrameUtils.getVerboseFragmentPlan(ctx, sql); @@ -419,9 +419,9 @@ public void testDecimalAddRule() throws Exception { plan = UtFrameUtils.getVerboseFragmentPlan(ctx, sql); Assert.assertTrue(plan.contains(" 2 <-> 1000000000000000000.000000000000000001")); - sql = "select dec_1_2 + dec_1_2 from test_decimal_type6"; + sql = "select dec_2_1 + dec_2_1 from test_decimal_type6"; plan = UtFrameUtils.getVerboseFragmentPlan(ctx, sql); - Assert.assertTrue(plan.contains("[11: cast, DECIMAL32(3,1), true] + [11: cast, DECIMAL32(3,1), true]")); + Assert.assertTrue(plan.contains("[11: cast, DECIMAL32(3,1), false] + [11: cast, DECIMAL32(3,1), false]")); } @Test @@ -494,7 +494,7 @@ public void testDecimalNullableProperties() throws Exception { plan = UtFrameUtils.getVerboseFragmentPlan(ctx, sql); System.out.println("plan = " + plan); Assert.assertTrue(plan.contains( - "round[(cast([2: dec_18_0, DECIMAL64(18,0), false] as DECIMAL128(18,0))); args: DECIMAL128; result: DECIMAL128(38,0); args nullable: true; result nullable: true]")); + "round[(cast([2: dec_18_0, DECIMAL64(18,0), false] as DECIMAL128(18,0))); args: DECIMAL128; result: DECIMAL128(38,0); args nullable: false; result nullable: true]")); } @Test diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/plan/DistributedEnvPlanWithCostTest.java b/fe/fe-core/src/test/java/com/starrocks/sql/plan/DistributedEnvPlanWithCostTest.java index fd21352d7570c..97ef5f594dba8 100644 --- a/fe/fe-core/src/test/java/com/starrocks/sql/plan/DistributedEnvPlanWithCostTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/sql/plan/DistributedEnvPlanWithCostTest.java @@ -539,7 +539,7 @@ public void testJoinOnExpression() throws Exception { String plan = getCostExplain(sql); assertContains(plan, "6:HASH JOIN\n" + " | join op: INNER JOIN (PARTITIONED)\n" + - " | equal join conjunct: [29: cast, BIGINT, true] = [30: add, BIGINT, true]\n" + + " | equal join conjunct: [29: cast, BIGINT, false] = [30: add, BIGINT, false]\n" + " | output columns: 14, 15, 20, 22\n" + " | cardinality: 600000000"); assertContains(plan, "|----5:EXCHANGE\n" + @@ -552,7 +552,7 @@ public void testJoinOnExpression() throws Exception { "GROUP BY l_shipmode, l_shipinstruct, o_orderdate, o_orderstatus;"; plan = getCostExplain(sql); Assert.assertTrue( - plan.contains("equal join conjunct: [29: multiply, BIGINT, true] = [30: add, BIGINT, true]\n" + + plan.contains("equal join conjunct: [29: multiply, BIGINT, false] = [30: add, BIGINT, false]\n" + " | output columns: 14, 15, 20, 22\n" + " | cardinality: 600000000")); } diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/plan/PlanTestBase.java b/fe/fe-core/src/test/java/com/starrocks/sql/plan/PlanTestBase.java index b096ae2057228..7b903f2268aca 100644 --- a/fe/fe-core/src/test/java/com/starrocks/sql/plan/PlanTestBase.java +++ b/fe/fe-core/src/test/java/com/starrocks/sql/plan/PlanTestBase.java @@ -1182,6 +1182,7 @@ public void runFileUnitTest(String filename, boolean debug) { e.printStackTrace(); } System.out.println("DEBUG MODE!"); + System.out.println("DEBUG FILE: " + debugFile.getPath()); } Pattern regex = Pattern.compile("\\[plan-(\\d+)]"); diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/plan/SetTest.java b/fe/fe-core/src/test/java/com/starrocks/sql/plan/SetTest.java index 2bb1cba5cfff2..0755be2ecc500 100644 --- a/fe/fe-core/src/test/java/com/starrocks/sql/plan/SetTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/sql/plan/SetTest.java @@ -303,8 +303,8 @@ public void testUnionNullConstant() throws Exception { " | output exprs:\n" + " | [7, DECIMAL64(12,2), true]\n" + " | child exprs:\n" + - " | [3, DECIMAL64(12,2), true]\n" + - " | [6, DECIMAL64(12,2), true]\n"); + " | [3, DECIMAL64(12,2), false]\n" + + " | [6, DECIMAL64(12,2), false]\n"); sql = "select count(*) from (select cast('1.2' as decimal(5,2)) as c1 union all " + "select cast('1.2' as decimal(10,0)) as c1) t group by t.c1"; @@ -313,8 +313,8 @@ public void testUnionNullConstant() throws Exception { " | output exprs:\n" + " | [7, DECIMAL64(12,2), true]\n" + " | child exprs:\n" + - " | [3, DECIMAL64(12,2), true]\n" + - " | [6, DECIMAL64(12,2), true]"); + " | [3, DECIMAL64(12,2), false]\n" + + " | [6, DECIMAL64(12,2), false]"); } @Test diff --git a/fe/fe-core/src/test/resources/sql/tpch-histogram-cost/q11.sql b/fe/fe-core/src/test/resources/sql/tpch-histogram-cost/q11.sql index cfaf6b15d712a..b392f3599935a 100644 --- a/fe/fe-core/src/test/resources/sql/tpch-histogram-cost/q11.sql +++ b/fe/fe-core/src/test/resources/sql/tpch-histogram-cost/q11.sql @@ -78,7 +78,7 @@ OutPut Exchange Id: 30 | cardinality: 1 | 10:AGGREGATE (update finalize) -| aggregate: sum[([20: expr, DOUBLE, true]); args: DOUBLE; result: DOUBLE; args nullable: true; result nullable: true] +| aggregate: sum[([20: expr, DOUBLE, false]); args: DOUBLE; result: DOUBLE; args nullable: false; result nullable: true] | group by: [1: PS_PARTKEY, INT, false] | cardinality: 3200000 | probe runtime filters: @@ -118,6 +118,7 @@ OutPut Exchange Id: 30 table: partsupp, rollup: partsupp preAggregation: on partitionsRatio=1/1, tabletsRatio=10/10 +tabletList=10181,10183,10185,10187,10189,10191,10193,10195,10197,10199 actualRows=0, avgRowSize=28.0 cardinality: 80000000 probe runtime filters: @@ -165,7 +166,7 @@ OutPut Partition: UNPARTITIONED OutPut Exchange Id: 22 21:AGGREGATE (update serialize) -| aggregate: sum[([41: expr, DOUBLE, true]); args: DOUBLE; result: DOUBLE; args nullable: true; result nullable: true] +| aggregate: sum[([41: expr, DOUBLE, false]); args: DOUBLE; result: DOUBLE; args nullable: false; result nullable: true] | cardinality: 1 | column statistics: | * sum-->[1.0, 9999000.0, 0.0, 8.0, 1.0] ESTIMATE @@ -198,6 +199,7 @@ OutPut Exchange Id: 22 table: partsupp, rollup: partsupp preAggregation: on partitionsRatio=1/1, tabletsRatio=10/10 +tabletList=10181,10183,10185,10187,10189,10191,10193,10195,10197,10199 actualRows=0, avgRowSize=20.0 cardinality: 80000000 probe runtime filters: @@ -239,6 +241,7 @@ OutPut Exchange Id: 18 table: supplier, rollup: supplier preAggregation: on partitionsRatio=1/1, tabletsRatio=1/1 +tabletList=10176 actualRows=0, avgRowSize=8.0 cardinality: 1000000 probe runtime filters: @@ -265,6 +268,7 @@ table: nation, rollup: nation preAggregation: on Predicates: [37: N_NAME, CHAR, false] = 'PERU' partitionsRatio=1/1, tabletsRatio=1/1 +tabletList=10250 actualRows=0, avgRowSize=29.0 cardinality: 1 column statistics: @@ -303,6 +307,7 @@ OutPut Exchange Id: 07 table: supplier, rollup: supplier preAggregation: on partitionsRatio=1/1, tabletsRatio=1/1 +tabletList=10176 actualRows=0, avgRowSize=8.0 cardinality: 1000000 probe runtime filters: @@ -329,6 +334,7 @@ table: nation, rollup: nation preAggregation: on Predicates: [16: N_NAME, CHAR, false] = 'PERU' partitionsRatio=1/1, tabletsRatio=1/1 +tabletList=10250 actualRows=0, avgRowSize=29.0 cardinality: 1 column statistics: From 93958921713dbb38a81f74b13574fae77fc6f1e8 Mon Sep 17 00:00:00 2001 From: silverbullet233 <3675229+silverbullet233@users.noreply.github.com> Date: Mon, 19 Dec 2022 14:46:45 +0800 Subject: [PATCH 3/4] remove uncessary tabletList --- .../src/test/resources/sql/tpch-histogram-cost/q11.sql | 5 ----- 1 file changed, 5 deletions(-) diff --git a/fe/fe-core/src/test/resources/sql/tpch-histogram-cost/q11.sql b/fe/fe-core/src/test/resources/sql/tpch-histogram-cost/q11.sql index b392f3599935a..3127c7af0218b 100644 --- a/fe/fe-core/src/test/resources/sql/tpch-histogram-cost/q11.sql +++ b/fe/fe-core/src/test/resources/sql/tpch-histogram-cost/q11.sql @@ -199,7 +199,6 @@ OutPut Exchange Id: 22 table: partsupp, rollup: partsupp preAggregation: on partitionsRatio=1/1, tabletsRatio=10/10 -tabletList=10181,10183,10185,10187,10189,10191,10193,10195,10197,10199 actualRows=0, avgRowSize=20.0 cardinality: 80000000 probe runtime filters: @@ -241,7 +240,6 @@ OutPut Exchange Id: 18 table: supplier, rollup: supplier preAggregation: on partitionsRatio=1/1, tabletsRatio=1/1 -tabletList=10176 actualRows=0, avgRowSize=8.0 cardinality: 1000000 probe runtime filters: @@ -268,7 +266,6 @@ table: nation, rollup: nation preAggregation: on Predicates: [37: N_NAME, CHAR, false] = 'PERU' partitionsRatio=1/1, tabletsRatio=1/1 -tabletList=10250 actualRows=0, avgRowSize=29.0 cardinality: 1 column statistics: @@ -307,7 +304,6 @@ OutPut Exchange Id: 07 table: supplier, rollup: supplier preAggregation: on partitionsRatio=1/1, tabletsRatio=1/1 -tabletList=10176 actualRows=0, avgRowSize=8.0 cardinality: 1000000 probe runtime filters: @@ -334,7 +330,6 @@ table: nation, rollup: nation preAggregation: on Predicates: [16: N_NAME, CHAR, false] = 'PERU' partitionsRatio=1/1, tabletsRatio=1/1 -tabletList=10250 actualRows=0, avgRowSize=29.0 cardinality: 1 column statistics: From a630da228c92f0b537b4272faa6edce116e5c9ac Mon Sep 17 00:00:00 2001 From: silverbullet233 <3675229+silverbullet233@users.noreply.github.com> Date: Mon, 19 Dec 2022 15:00:45 +0800 Subject: [PATCH 4/4] remove uncessary tabletList --- fe/fe-core/src/test/resources/sql/tpch-histogram-cost/q11.sql | 1 - 1 file changed, 1 deletion(-) diff --git a/fe/fe-core/src/test/resources/sql/tpch-histogram-cost/q11.sql b/fe/fe-core/src/test/resources/sql/tpch-histogram-cost/q11.sql index 3127c7af0218b..eff05a203f96f 100644 --- a/fe/fe-core/src/test/resources/sql/tpch-histogram-cost/q11.sql +++ b/fe/fe-core/src/test/resources/sql/tpch-histogram-cost/q11.sql @@ -118,7 +118,6 @@ OutPut Exchange Id: 30 table: partsupp, rollup: partsupp preAggregation: on partitionsRatio=1/1, tabletsRatio=10/10 -tabletList=10181,10183,10185,10187,10189,10191,10193,10195,10197,10199 actualRows=0, avgRowSize=28.0 cardinality: 80000000 probe runtime filters: