From fb5b412698de863ff8b2fdca225ae889be479962 Mon Sep 17 00:00:00 2001 From: starocean999 <40539150+starocean999@users.noreply.github.com> Date: Fri, 21 Jul 2023 11:34:56 +0800 Subject: [PATCH] [fix](planner)fix bug of pushing conjuncts into inlineview (#21962) 1. markConstantConjunct method shouldn't change the input conjunct 2. Use Expr's comeFrom method to check if the pushed expr is one of the group by exprs, this is the correct way to check if the conjunct can be pushed down through the agg node. 3. migrateConstantConjuncts should substitute the conjuncts using inlineViewRef's analyzer to make the analyzer recognize the column in the conjuncts in the following analyze phase --- .../org/apache/doris/analysis/Analyzer.java | 4 +- .../doris/planner/SingleNodePlanner.java | 14 +++-- .../test_push_conjuncts_inlineview.groovy | 54 +++++++++++++++++++ 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java index 19255873c0..1e44278eef 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java @@ -1891,7 +1891,9 @@ public class Analyzer { // aliases and having it analyzed is needed for the following EvalPredicate() call conjunct.analyze(this); } - Expr newConjunct = conjunct.getResultValue(true); + // getResultValue will modify the conjunct internally + // we have to use a clone to keep conjunct unchanged + Expr newConjunct = conjunct.clone().getResultValue(true); newConjunct = FoldConstantsRule.INSTANCE.apply(newConjunct, this, null); if (newConjunct instanceof BoolLiteral || newConjunct instanceof NullLiteral) { boolean evalResult = true; diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java b/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java index 9e041302c2..0e0fceb4c4 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java +++ b/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java @@ -1860,15 +1860,21 @@ public class SingleNodePlanner { return; } - List newConjuncts = cloneExprs(conjuncts); final QueryStmt stmt = inlineViewRef.getViewStmt(); final Analyzer viewAnalyzer = inlineViewRef.getAnalyzer(); viewAnalyzer.markConjunctsAssigned(conjuncts); + // even if the conjuncts are constant, they may contains slotRef + // for example: case when slotRef is null then 0 else 1 + // we need substitute the conjuncts using inlineViewRef's analyzer + // otherwise, when analyzing the conjunct in the inline view + // the analyzer is not able to find the column because it comes from outside + List newConjuncts = + Expr.substituteList(conjuncts, inlineViewRef.getSmap(), viewAnalyzer, false); if (stmt instanceof SelectStmt) { final SelectStmt select = (SelectStmt) stmt; if (select.getAggInfo() != null) { viewAnalyzer.registerConjuncts(newConjuncts, select.getAggInfo().getOutputTupleId().asList()); - } else if (select.getTableRefs().size() > 1) { + } else if (select.getTableRefs().size() > 0) { for (int i = select.getTableRefs().size() - 1; i >= 0; i--) { viewAnalyzer.registerConjuncts(newConjuncts, select.getTableRefs().get(i).getDesc().getId().asList()); @@ -2799,7 +2805,9 @@ public class SingleNodePlanner { } GroupByClause groupByClause = stmt.getGroupByClause(); List exprs = groupByClause.getGroupingExprs(); - if (!exprs.contains(sourceExpr)) { + final Expr srcExpr = sourceExpr; + if (!exprs.stream().anyMatch(expr -> expr.comeFrom(srcExpr))) { + // the sourceExpr doesn't come from any of the group by exprs isAllSlotReferToGroupBys = false; break; } diff --git a/regression-test/suites/correctness_p0/test_push_conjuncts_inlineview.groovy b/regression-test/suites/correctness_p0/test_push_conjuncts_inlineview.groovy index 80a35194f6..462dfed26d 100644 --- a/regression-test/suites/correctness_p0/test_push_conjuncts_inlineview.groovy +++ b/regression-test/suites/correctness_p0/test_push_conjuncts_inlineview.groovy @@ -60,6 +60,60 @@ suite("test_push_conjuncts_inlineview") { contains "4:VSELECT" } +explain { + sql("""SELECT * + FROM + (SELECT `a_key` AS `a_key` + FROM + (SELECT `b`.`a_key` AS `a_key` + FROM + (SELECT `a`.`a_key` AS `a_key` + FROM `push_conjunct_table` a) b + GROUP BY 1 ) t2 ) t1 + WHERE a_key = '123';""") + notContains "having" + contains "= '123'" + } + +sql """ + WITH ttt AS + (SELECT c1, + c2, + c3, + c4, + c5, + c6, + c7 + FROM + (SELECT '10000003' c1, '0816ffk' c2, '1' c3, 1416.0800 c4, '0816ffk' c5, '2023-07-03 15:36:36' c6, 1 c7 ) a + WHERE c7 = 1 ) + SELECT dd.c1, + dd.d1 + FROM + (SELECT src.c1, + + CASE + WHEN IFNULL(src.c3,'') = '' + OR src.c3 = '3' THEN + '-1' + WHEN src.c4 = 0 THEN + '0' + WHEN src.c4 <= 200 THEN + '1' + WHEN src.c4 > 200 + AND src.c4 <= 500 THEN + '2' + WHEN src.c4 > 500 + AND src.c4 <= 1000 THEN + '3' + ELSE '4' + END AS d1 + FROM ttt src + WHERE src.c1 = '10000003' + GROUP BY src.c1, d1 ) dd + WHERE dd.d1 IN ('-1'); +""" + sql """ DROP TABLE IF EXISTS `push_conjunct_table` """ }