From 215f402df79321dec86ac467de75afb2f120fd0a Mon Sep 17 00:00:00 2001 From: minghong Date: Wed, 10 Apr 2024 10:24:39 +0800 Subject: [PATCH] [fix](nereids)when clause cannot be regarded as common sub expression (#33358) * when clause cannot be regarded as common sub expression --- .../processor/post/CommonSubExpressionOpt.java | 14 +++++++++++--- regression-test/data/tpch_sf0.1_p1/sql/cse.out | 10 +++++++++- .../suites/tpch_sf0.1_p1/sql/cse.groovy | 7 ++++++- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/CommonSubExpressionOpt.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/CommonSubExpressionOpt.java index dfaf2de757..194116cf31 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/CommonSubExpressionOpt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/CommonSubExpressionOpt.java @@ -22,6 +22,7 @@ import org.apache.doris.nereids.trees.expressions.Alias; import org.apache.doris.nereids.trees.expressions.Expression; import org.apache.doris.nereids.trees.expressions.NamedExpression; import org.apache.doris.nereids.trees.expressions.Slot; +import org.apache.doris.nereids.trees.expressions.WhenClause; import org.apache.doris.nereids.trees.expressions.visitor.DefaultExpressionRewriter; import org.apache.doris.nereids.trees.plans.Plan; import org.apache.doris.nereids.trees.plans.physical.PhysicalProject; @@ -82,9 +83,16 @@ public class CommonSubExpressionOpt extends PlanPostProcessor { Set exprsInDepth = CommonSubExpressionCollector .getExpressionsFromDepthMap(i, collector.commonExprByDepth); exprsInDepth.forEach(expr -> { - Expression rewritten = expr.accept(ExpressionReplacer.INSTANCE, aliasMap); - Alias alias = new Alias(rewritten); - aliasMap.put(expr, alias); + if (!(expr instanceof WhenClause)) { + // case whenClause1 whenClause2 END + // whenClause should not be regarded as common-sub-expression, because + // cse will be replaced by a slot, after rewrite the case clause becomes: + // 'case slot whenClause2 END' + // This is illegal. + Expression rewritten = expr.accept(ExpressionReplacer.INSTANCE, aliasMap); + Alias alias = new Alias(rewritten); + aliasMap.put(expr, alias); + } }); layer.addAll(aliasMap.values()); multiLayers.add(layer); diff --git a/regression-test/data/tpch_sf0.1_p1/sql/cse.out b/regression-test/data/tpch_sf0.1_p1/sql/cse.out index 5ab4465566..b65ac322a3 100644 --- a/regression-test/data/tpch_sf0.1_p1/sql/cse.out +++ b/regression-test/data/tpch_sf0.1_p1/sql/cse.out @@ -27,4 +27,12 @@ 12093 13093 14093 15093 -- !cse_4 -- -12093 13093 14093 15093 \ No newline at end of file +12093 13093 14093 15093 + +-- !cse_5 -- +1 2 +2 3 +2 5 +5 6 +6 7 + diff --git a/regression-test/suites/tpch_sf0.1_p1/sql/cse.groovy b/regression-test/suites/tpch_sf0.1_p1/sql/cse.groovy index 698dbd3e5d..d5989cf68b 100644 --- a/regression-test/suites/tpch_sf0.1_p1/sql/cse.groovy +++ b/regression-test/suites/tpch_sf0.1_p1/sql/cse.groovy @@ -45,5 +45,10 @@ suite('cse') { qt_cse_4 """select sum(s_nationkey),sum(s_nationkey) + count(1) ,sum(s_nationkey) + 2 * count(1) , sum(s_nationkey) + 3 * count(1) from supplier ;""" - + // when clause cannot be regarded as common-sub-expression. + // if "when r_regionkey=1 then 0" is regarded as a common-sub-expression, but if it will be replaced by a slot, and hence the case clause + // become syntax illegal: case cseSlot when r_regionkey=2 the 1 else .... + qt_cse_5 """select (case r_regionkey when 1 then 0 when 2 then 1 else r_regionkey+1 END) + 1 As x, + (case r_regionkey when 1 then 0 when 2 then 3 else r_regionkey+1 END) + 2 as y + from region order by x, y;""" }