From 350881b2d0a2f7c352fdcf7b23df2144706a38d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B0=A2=E5=81=A5?= Date: Fri, 3 Nov 2023 16:30:12 +0800 Subject: [PATCH] [fix](Planner): don't push down isNull predicate into view (#26288) We can't push down `isNull` predicate into view --- .../java/org/apache/doris/analysis/BinaryPredicate.java | 4 ++-- .../main/java/org/apache/doris/analysis/CastExpr.java | 4 ++-- .../org/apache/doris/analysis/CompoundPredicate.java | 4 ++-- .../src/main/java/org/apache/doris/analysis/Expr.java | 4 ++-- .../main/java/org/apache/doris/analysis/InPredicate.java | 4 ++-- .../java/org/apache/doris/analysis/IsNullPredicate.java | 8 +++++--- .../src/main/java/org/apache/doris/analysis/SlotRef.java | 6 +++--- .../java/org/apache/doris/analysis/VariableExpr.java | 2 +- .../data/query_p0/literal_view/lietral_test.out | 5 +++++ .../suites/query_p0/literal_view/lietral_test.groovy | 9 +++++++++ 10 files changed, 33 insertions(+), 17 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/BinaryPredicate.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/BinaryPredicate.java index bce4807357bdca..371915c501f3de 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/BinaryPredicate.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/BinaryPredicate.java @@ -720,8 +720,8 @@ public static BinaryPredicate read(DataInput in) throws IOException { } @Override - public Expr getResultValue(boolean inView) throws AnalysisException { - recursiveResetChildrenResult(inView); + public Expr getResultValue(boolean forPushDownPredicatesToView) throws AnalysisException { + recursiveResetChildrenResult(forPushDownPredicatesToView); final Expr leftChildValue = getChild(0); final Expr rightChildValue = getChild(1); if (!(leftChildValue instanceof LiteralExpr) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java index cdb55cec3ca144..dc21eff6563f14 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java @@ -399,8 +399,8 @@ public boolean canHashPartition() { } @Override - public Expr getResultValue(boolean inView) throws AnalysisException { - recursiveResetChildrenResult(inView); + public Expr getResultValue(boolean forPushDownPredicatesToView) throws AnalysisException { + recursiveResetChildrenResult(forPushDownPredicatesToView); final Expr value = children.get(0); if (!(value instanceof LiteralExpr)) { return this; diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/CompoundPredicate.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/CompoundPredicate.java index e20ec98b756b36..78e28da6ed9c51 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CompoundPredicate.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/CompoundPredicate.java @@ -230,8 +230,8 @@ public static boolean isOr(Expr expr) { } @Override - public Expr getResultValue(boolean inView) throws AnalysisException { - recursiveResetChildrenResult(inView); + public Expr getResultValue(boolean forPushDownPredicatesToView) throws AnalysisException { + recursiveResetChildrenResult(forPushDownPredicatesToView); boolean compoundResult = false; if (op == Operator.NOT) { final Expr childValue = getChild(0); diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/Expr.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/Expr.java index d73fa1ee9a2cb2..ecf1f29f97a906 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/Expr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/Expr.java @@ -2177,10 +2177,10 @@ public boolean supportSerializable() { } - protected void recursiveResetChildrenResult(boolean inView) throws AnalysisException { + protected void recursiveResetChildrenResult(boolean forPushDownPredicatesToView) throws AnalysisException { for (int i = 0; i < children.size(); i++) { final Expr child = children.get(i); - final Expr newChild = child.getResultValue(inView); + final Expr newChild = child.getResultValue(forPushDownPredicatesToView); if (newChild != child) { setChild(i, newChild); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/InPredicate.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/InPredicate.java index c110b2e6ea3321..3a08359a75ad99 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/InPredicate.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/InPredicate.java @@ -308,8 +308,8 @@ public String toString() { } @Override - public Expr getResultValue(boolean inView) throws AnalysisException { - recursiveResetChildrenResult(inView); + public Expr getResultValue(boolean forPushDownPredicatesToView) throws AnalysisException { + recursiveResetChildrenResult(forPushDownPredicatesToView); final Expr leftChildValue = getChild(0); if (!(leftChildValue instanceof LiteralExpr) || !isLiteralChildren()) { return this; diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/IsNullPredicate.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/IsNullPredicate.java index 3542160741d184..c67ca1b06029db 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/IsNullPredicate.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/IsNullPredicate.java @@ -155,10 +155,12 @@ public boolean isNullable() { * fix issue 6390 */ @Override - public Expr getResultValue(boolean inView) throws AnalysisException { - recursiveResetChildrenResult(!inView); + public Expr getResultValue(boolean forPushDownPredicatesToView) throws AnalysisException { + // Don't push down predicate to view for is null predicate because the value can contain null + // after outer join + recursiveResetChildrenResult(!forPushDownPredicatesToView); final Expr childValue = getChild(0); - if (inView || !(childValue instanceof LiteralExpr)) { + if (forPushDownPredicatesToView || !(childValue instanceof LiteralExpr)) { return this; } return childValue instanceof NullLiteral ? new BoolLiteral(!isNotNull) : new BoolLiteral(isNotNull); diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java index ae8eeabe63a533..cf945f540d0597 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java @@ -607,8 +607,8 @@ public boolean matchExprs(List exprs, SelectStmt stmt, boolean ignoreAlias } @Override - public Expr getResultValue(boolean foldSlot) throws AnalysisException { - if (!foldSlot) { + public Expr getResultValue(boolean forPushDownPredicatesToView) throws AnalysisException { + if (!forPushDownPredicatesToView) { return this; } if (!isConstant() || desc == null) { @@ -620,7 +620,7 @@ public Expr getResultValue(boolean foldSlot) throws AnalysisException { } Expr expr = exprs.get(0); if (expr instanceof SlotRef) { - return expr.getResultValue(foldSlot); + return expr.getResultValue(forPushDownPredicatesToView); } if (expr.isConstant()) { return expr; diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/VariableExpr.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/VariableExpr.java index 8e657d6418dacf..093a7861173c0a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/VariableExpr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/VariableExpr.java @@ -137,7 +137,7 @@ public Expr getLiteralExpr() { } @Override - public Expr getResultValue(boolean inView) throws AnalysisException { + public Expr getResultValue(boolean forPushDownPredicatesToView) throws AnalysisException { if (!Strings.isNullOrEmpty(name) && VariableVarConverters.hasConverter(name)) { // Return the string type here so that it can correctly match the subsequent function signature. // And we also set `beConverted` to session variable name in StringLiteral, so that it can be cast back diff --git a/regression-test/data/query_p0/literal_view/lietral_test.out b/regression-test/data/query_p0/literal_view/lietral_test.out index daa91d59cd1c1f..0c46e0b270fe6e 100644 --- a/regression-test/data/query_p0/literal_view/lietral_test.out +++ b/regression-test/data/query_p0/literal_view/lietral_test.out @@ -1,5 +1,10 @@ -- This file is automatically generated. You should know what you did if you want to edit this -- !sql -- +-- !left -- +1 doris 10 \N +2 spark 2 \N +3 flink 20 \N + -- !sql1 -- diff --git a/regression-test/suites/query_p0/literal_view/lietral_test.groovy b/regression-test/suites/query_p0/literal_view/lietral_test.groovy index d1a1b51fd69b10..b19f33e2939685 100644 --- a/regression-test/suites/query_p0/literal_view/lietral_test.groovy +++ b/regression-test/suites/query_p0/literal_view/lietral_test.groovy @@ -106,6 +106,15 @@ suite("literal_view_test") { insert into test_insert values (1,'doris',10),(2,'spark',2),(3,'flink',20); """ + sql "set enable_nereids_planner=false" + order_qt_left """select * + from test_insert + left join (select 1 as v1) t1 + on false + where t1.v1 is null + """ + sql "set enable_nereids_planner=true" + qt_sql1 """ select id, name from (