This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-doris.git
The following commit(s) were added to refs/heads/master by this push:
new 101628c [Bug] Fix bug of predicate pushdown logic (#3475)
101628c is described below
commit 101628c81314f1597e1018faa41414b7a19037d8
Author: Mingyu Chen <morningman....@gmail.com>
AuthorDate: Wed May 6 15:15:37 2020 +0800
[Bug] Fix bug of predicate pushdown logic (#3475)
When there is subquery in where clause, the query will be rewritten to join
operation.
And some auxiliary binary predicates will be generated. These binary
predicates
will not go through the ExprRewriteRule, so they are not normalized as
"column to the left and constant to the right" format.
We need to take this case into account so that the `canPushDownPredicate()`
judgement
will not throw exception.
---
.../java/org/apache/doris/analysis/Predicate.java | 21 ++++++++++-
.../org/apache/doris/planner/QueryPlanTest.java | 44 +++++++++++++++++++++-
2 files changed, 61 insertions(+), 4 deletions(-)
diff --git a/fe/src/main/java/org/apache/doris/analysis/Predicate.java
b/fe/src/main/java/org/apache/doris/analysis/Predicate.java
index d345cdb..3e2c22b 100644
--- a/fe/src/main/java/org/apache/doris/analysis/Predicate.java
+++ b/fe/src/main/java/org/apache/doris/analysis/Predicate.java
@@ -17,12 +17,13 @@
package org.apache.doris.analysis;
-import com.google.common.base.Preconditions;
import org.apache.doris.catalog.Type;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.Pair;
import org.apache.doris.common.Reference;
+import com.google.common.base.Preconditions;
+
public abstract class Predicate extends Expr {
protected boolean isEqJoinConjunct;
@@ -109,7 +110,23 @@ public abstract class Predicate extends Expr {
// because isSingleColumnPredicate
Preconditions.checkState(right != null);
- Preconditions.checkState(right.isConstant());
+
+ // ATTN(cmy): Usually, the BinaryPredicate in the query will
be rewritten through ExprRewriteRule,
+ // and all SingleColumnPredicate will be rewritten as "column
on the left and the constant on the right".
+ // So usually the right child is constant.
+ //
+ // But if there is a subquery in where clause, the planner
will rewrite the subquery to join.
+ // During the rewrite, some auxiliary BinaryPredicate will be
automatically generated,
+ // and these BinaryPredicates will not go through
ExprRewriteRule.
+ // As a result, these BinaryPredicates may be as "column on
the right and the constant on the left".
+ // Example can be found in QueryPlanTest.java ->
testJoinPredicateTransitivityWithSubqueryInWhereClause().
+ //
+ // Because our current planner implementation is very
error-prone, so when this happens,
+ // we simply assume that these kind of BinaryPredicates cannot
be pushed down,
+ // to ensure that this change will not affect other query
plans.
+ if (!right.isConstant()) {
+ return false;
+ }
return right instanceof LiteralExpr;
}
diff --git a/fe/src/test/java/org/apache/doris/planner/QueryPlanTest.java
b/fe/src/test/java/org/apache/doris/planner/QueryPlanTest.java
index 53a3c15..c192d18 100644
--- a/fe/src/test/java/org/apache/doris/planner/QueryPlanTest.java
+++ b/fe/src/test/java/org/apache/doris/planner/QueryPlanTest.java
@@ -242,6 +242,32 @@ public class QueryPlanTest {
"PROPERTIES (\n" +
" \"replication_num\" = \"1\"\n" +
");");
+
+ createTable("CREATE TABLE test.`pushdown_test` (\n" +
+ " `k1` tinyint(4) NULL COMMENT \"\",\n" +
+ " `k2` smallint(6) NULL COMMENT \"\",\n" +
+ " `k3` int(11) NULL COMMENT \"\",\n" +
+ " `k4` bigint(20) NULL COMMENT \"\",\n" +
+ " `k5` decimal(9, 3) NULL COMMENT \"\",\n" +
+ " `k6` char(5) NULL COMMENT \"\",\n" +
+ " `k10` date NULL COMMENT \"\",\n" +
+ " `k11` datetime NULL COMMENT \"\",\n" +
+ " `k7` varchar(20) NULL COMMENT \"\",\n" +
+ " `k8` double MAX NULL COMMENT \"\",\n" +
+ " `k9` float SUM NULL COMMENT \"\"\n" +
+ ") ENGINE=OLAP\n" +
+ "AGGREGATE KEY(`k1`, `k2`, `k3`, `k4`, `k5`, `k6`, `k10`,
`k11`, `k7`)\n" +
+ "COMMENT \"OLAP\"\n" +
+ "PARTITION BY RANGE(`k1`)\n" +
+ "(PARTITION p1 VALUES [(\"-128\"), (\"-64\")),\n" +
+ "PARTITION p2 VALUES [(\"-64\"), (\"0\")),\n" +
+ "PARTITION p3 VALUES [(\"0\"), (\"64\")))\n" +
+ "DISTRIBUTED BY HASH(`k1`) BUCKETS 5\n" +
+ "PROPERTIES (\n" +
+ "\"replication_num\" = \"1\",\n" +
+ "\"in_memory\" = \"false\",\n" +
+ "\"storage_format\" = \"DEFAULT\"\n" +
+ ");");
}
@AfterClass
@@ -532,7 +558,7 @@ public class QueryPlanTest {
}
@Test
- public void testJoinPredicateTransitivity() throws Exception {
+ public void testJoinPredicateTransitivity() throws Exception {
connectContext.setDatabase("default_cluster:test");
// test left join : left table where binary predicate
@@ -624,10 +650,24 @@ public class QueryPlanTest {
// test inner join: right table join predicate, both push down left
table and right table
sql = "select *\n from join1\n" +
"join join2 on join1.id = join2.id\n" +
- "and join2.id > 1;";
+ "and 1 < join2.id;";
explainString = UtFrameUtils.getSQLPlanOrErrorMsg(connectContext,
"explain " + sql);
System.out.println(explainString);
Assert.assertTrue(explainString.contains("PREDICATES: `join1`.`id` >
1"));
Assert.assertTrue(explainString.contains("PREDICATES: `join2`.`id` >
1"));
}
+
+ @Test
+ public void testJoinPredicateTransitivityWithSubqueryInWhereClause()
throws Exception {
+ connectContext.setDatabase("default_cluster:test");
+ String sql = "SELECT *\n" +
+ "FROM test.pushdown_test\n" +
+ "WHERE 0 < (\n" +
+ " SELECT MAX(k9)\n" +
+ " FROM test.pushdown_test);";
+ String explainString =
UtFrameUtils.getSQLPlanOrErrorMsg(connectContext, "explain " + sql);
+ Assert.assertTrue(explainString.contains("PLAN FRAGMENT"));
+ Assert.assertTrue(explainString.contains("CROSS JOIN"));
+ Assert.assertTrue(!explainString.contains("PREDICATES"));
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
