You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by mo...@apache.org on 2023/01/23 14:18:00 UTC
[doris] 02/05: [fix](fe)fix anti join bug (#15955)
This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch branch-1.2-lts
in repository https://gitbox.apache.org/repos/asf/doris.git
commit 5e97bce8c01ff98118c7625074aa4a9be5882d60
Author: starocean999 <40...@users.noreply.github.com>
AuthorDate: Tue Jan 17 20:25:00 2023 +0800
[fix](fe)fix anti join bug (#15955)
* [fix](fe)fix anti join bug
* fix fe ut
---
.../java/org/apache/doris/analysis/Analyzer.java | 93 ++++++++++++++++------
.../org/apache/doris/analysis/JoinOperator.java | 13 +++
.../apache/doris/planner/SingleNodePlanner.java | 9 +--
.../java/org/apache/doris/planner/PlannerTest.java | 2 +-
.../org/apache/doris/planner/QueryPlanTest.java | 2 +-
regression-test/data/query_p0/join/test_join.out | 80 +++++++++++++++++++
.../suites/query_p0/join/test_join.groovy | 10 ++-
7 files changed, 178 insertions(+), 31 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 946718b2c6..649f054e39 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
@@ -358,7 +358,9 @@ public class Analyzer {
// corresponding value could be an empty list. There is no entry for non-outer joins.
public final Map<TupleId, List<ExprId>> conjunctsByOjClause = Maps.newHashMap();
- public final Map<TupleId, List<ExprId>> conjunctsByAntiJoinClause = Maps.newHashMap();
+ public final Map<TupleId, List<ExprId>> conjunctsByAntiJoinNullAwareClause = Maps.newHashMap();
+
+ public final Map<TupleId, List<ExprId>> conjunctsBySemiAntiJoinNoNullAwareClause = Maps.newHashMap();
// map from registered conjunct to its containing outer join On clause (represented
// by its right-hand side table ref); only conjuncts that can only be correctly
@@ -1387,10 +1389,27 @@ public class Analyzer {
* Return all unassigned conjuncts of the anti join referenced by
* right-hand side table ref.
*/
- public List<Expr> getUnassignedAntiJoinConjuncts(TableRef ref) {
- Preconditions.checkState(ref.getJoinOp().isAntiJoin());
+ public List<Expr> getUnassignedAntiJoinNullAwareConjuncts(TableRef ref) {
+ Preconditions.checkState(ref.getJoinOp().isAntiJoinNullAware());
+ List<Expr> result = Lists.newArrayList();
+ List<ExprId> candidates = globalState.conjunctsByAntiJoinNullAwareClause.get(ref.getId());
+ if (candidates == null) {
+ return result;
+ }
+ for (ExprId conjunctId : candidates) {
+ if (!globalState.assignedConjuncts.contains(conjunctId)) {
+ Expr e = globalState.conjuncts.get(conjunctId);
+ Preconditions.checkState(e != null);
+ result.add(e);
+ }
+ }
+ return result;
+ }
+
+ public List<Expr> getUnassignedSemiAntiJoinNoNullAwareConjuncts(TableRef ref) {
+ Preconditions.checkState(ref.getJoinOp().isSemiOrAntiJoinNoNullAware());
List<Expr> result = Lists.newArrayList();
- List<ExprId> candidates = globalState.conjunctsByAntiJoinClause.get(ref.getId());
+ List<ExprId> candidates = globalState.conjunctsBySemiAntiJoinNoNullAwareClause.get(ref.getId());
if (candidates == null) {
return result;
}
@@ -1480,6 +1499,30 @@ public class Analyzer {
return (tblRef.getJoinOp().isAntiJoin()) ? tblRef : null;
}
+ public boolean isAntiJoinedNullAwareConjunct(Expr e) {
+ return getAntiJoinNullAwareRef(e) != null;
+ }
+
+ private TableRef getAntiJoinNullAwareRef(Expr e) {
+ TableRef tblRef = globalState.sjClauseByConjunct.get(e.getId());
+ if (tblRef == null) {
+ return null;
+ }
+ return (tblRef.getJoinOp().isAntiJoinNullAware()) ? tblRef : null;
+ }
+
+ public boolean isAntiJoinedNoNullAwareConjunct(Expr e) {
+ return getAntiJoinNoNullAwareRef(e) != null;
+ }
+
+ public TableRef getAntiJoinNoNullAwareRef(Expr e) {
+ TableRef tblRef = globalState.sjClauseByConjunct.get(e.getId());
+ if (tblRef == null) {
+ return null;
+ }
+ return (tblRef.getJoinOp().isAntiJoinNoNullAware()) ? tblRef : null;
+ }
+
public boolean isFullOuterJoined(TupleId tid) {
return globalState.fullOuterJoinedTupleIds.containsKey(tid);
}
@@ -1697,11 +1740,14 @@ public class Analyzer {
globalState.ojClauseByConjunct.put(conjunct.getId(), rhsRef);
ojConjuncts.add(conjunct.getId());
}
- if (rhsRef.getJoinOp().isSemiJoin()) {
+ if (rhsRef.getJoinOp().isSemiAntiJoin()) {
globalState.sjClauseByConjunct.put(conjunct.getId(), rhsRef);
- if (rhsRef.getJoinOp().isAntiJoin()) {
- globalState.conjunctsByAntiJoinClause.computeIfAbsent(rhsRef.getId(), k -> Lists.newArrayList())
- .add(conjunct.getId());
+ if (rhsRef.getJoinOp().isAntiJoinNullAware()) {
+ globalState.conjunctsByAntiJoinNullAwareClause.computeIfAbsent(rhsRef.getId(),
+ k -> Lists.newArrayList()).add(conjunct.getId());
+ } else {
+ globalState.conjunctsBySemiAntiJoinNoNullAwareClause.computeIfAbsent(rhsRef.getId(),
+ k -> Lists.newArrayList()).add(conjunct.getId());
}
}
if (rhsRef.getJoinOp().isInnerJoin()) {
@@ -1721,7 +1767,7 @@ public class Analyzer {
*/
private void markConstantConjunct(Expr conjunct, boolean fromHavingClause)
throws AnalysisException {
- if (!conjunct.isConstant() || isOjConjunct(conjunct) || isAntiJoinedConjunct(conjunct)) {
+ if (!conjunct.isConstant() || isOjConjunct(conjunct)) {
return;
}
if ((!fromHavingClause && !hasEmptySpjResultSet)
@@ -1738,24 +1784,25 @@ public class Analyzer {
conjunct.analyze(this);
}
final Expr newConjunct = conjunct.getResultValue();
- if (newConjunct instanceof BoolLiteral) {
- final BoolLiteral value = (BoolLiteral) newConjunct;
- if (!value.getValue()) {
- if (fromHavingClause) {
- hasEmptyResultSet = true;
- } else {
- hasEmptySpjResultSet = true;
- }
+ if (newConjunct instanceof BoolLiteral || newConjunct instanceof NullLiteral) {
+ boolean evalResult = true;
+ if (newConjunct instanceof BoolLiteral) {
+ evalResult = ((BoolLiteral) newConjunct).getValue();
+ } else {
+ evalResult = false;
}
- markConjunctAssigned(conjunct);
- }
- if (newConjunct instanceof NullLiteral) {
if (fromHavingClause) {
- hasEmptyResultSet = true;
+ hasEmptyResultSet = !evalResult;
} else {
- hasEmptySpjResultSet = true;
+ if (isAntiJoinedNoNullAwareConjunct(conjunct)) {
+ hasEmptySpjResultSet = evalResult;
+ } else {
+ hasEmptySpjResultSet = !evalResult;
+ }
+ }
+ if (hasEmptyResultSet || hasEmptySpjResultSet) {
+ markConjunctAssigned(conjunct);
}
- markConjunctAssigned(conjunct);
}
} catch (AnalysisException ex) {
throw new AnalysisException("Error evaluating \"" + conjunct.toSql() + "\"", ex);
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/JoinOperator.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/JoinOperator.java
index de33961db1..08c0321b25 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/JoinOperator.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/JoinOperator.java
@@ -71,6 +71,19 @@ public enum JoinOperator {
|| this == JoinOperator.NULL_AWARE_LEFT_ANTI_JOIN;
}
+ public boolean isSemiOrAntiJoinNoNullAware() {
+ return this == JoinOperator.LEFT_SEMI_JOIN || this == JoinOperator.LEFT_ANTI_JOIN
+ || this == JoinOperator.RIGHT_SEMI_JOIN || this == JoinOperator.RIGHT_ANTI_JOIN;
+ }
+
+ public boolean isAntiJoinNullAware() {
+ return this == JoinOperator.NULL_AWARE_LEFT_ANTI_JOIN;
+ }
+
+ public boolean isAntiJoinNoNullAware() {
+ return this == JoinOperator.LEFT_ANTI_JOIN || this == JoinOperator.RIGHT_ANTI_JOIN;
+ }
+
public boolean isLeftSemiJoin() {
return this.thriftJoinOp == TJoinOp.LEFT_SEMI_JOIN;
}
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 e6ab97d2a4..854da5ddcc 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
@@ -2069,11 +2069,10 @@ public class SingleNodePlanner {
// Also assign conjuncts from On clause. All remaining unassigned conjuncts
// that can be evaluated by this join are assigned in createSelectPlan().
ojConjuncts = analyzer.getUnassignedOjConjuncts(innerRef);
- } else if (innerRef.getJoinOp().isAntiJoin()) {
- ojConjuncts = analyzer.getUnassignedAntiJoinConjuncts(innerRef);
- } else if (innerRef.getJoinOp().isSemiJoin()) {
- final List<TupleId> tupleIds = innerRef.getAllTupleIds();
- ojConjuncts = analyzer.getUnassignedConjuncts(tupleIds, false);
+ } else if (innerRef.getJoinOp().isAntiJoinNullAware()) {
+ ojConjuncts = analyzer.getUnassignedAntiJoinNullAwareConjuncts(innerRef);
+ } else if (innerRef.getJoinOp().isSemiOrAntiJoinNoNullAware()) {
+ ojConjuncts = analyzer.getUnassignedSemiAntiJoinNoNullAwareConjuncts(innerRef);
}
analyzer.markConjunctsAssigned(ojConjuncts);
if (eqJoinConjuncts.isEmpty() || innerRef.isMark()) {
diff --git a/fe/fe-core/src/test/java/org/apache/doris/planner/PlannerTest.java b/fe/fe-core/src/test/java/org/apache/doris/planner/PlannerTest.java
index 3d2293c670..2b5e0c3266 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/planner/PlannerTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/planner/PlannerTest.java
@@ -405,7 +405,7 @@ public class PlannerTest extends TestWithFeService {
compare.accept("select * from db1.tbl2 where k1 = 2.0", "select * from db1.tbl2 where k1 = 2");
compare.accept("select * from db1.tbl2 where k1 = 2.1", "select * from db1.tbl2 where 2 = 2.1");
compare.accept("select * from db1.tbl2 where k1 != 2.0", "select * from db1.tbl2 where k1 != 2");
- compare.accept("select * from db1.tbl2 where k1 != 2.1", "select * from db1.tbl2");
+ compare.accept("select * from db1.tbl2 where k1 != 2.1", "select * from db1.tbl2 where TRUE");
compare.accept("select * from db1.tbl2 where k1 <= 2.0", "select * from db1.tbl2 where k1 <= 2");
compare.accept("select * from db1.tbl2 where k1 <= 2.1", "select * from db1.tbl2 where k1 <= 2");
compare.accept("select * from db1.tbl2 where k1 >= 2.0", "select * from db1.tbl2 where k1 >= 2");
diff --git a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java
index 4f41e70c45..9d8e560c11 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java
@@ -1025,7 +1025,7 @@ public class QueryPlanTest extends TestWithFeService {
String explainString = getSQLPlanOrErrorMsg("explain " + sql);
Assert.assertTrue(explainString.contains("PLAN FRAGMENT"));
Assert.assertTrue(explainString.contains("NESTED LOOP JOIN"));
- Assert.assertTrue(!explainString.contains("PREDICATES"));
+ Assert.assertTrue(!explainString.contains("PREDICATES") || explainString.contains("PREDICATES: TRUE"));
}
@Test
diff --git a/regression-test/data/query_p0/join/test_join.out b/regression-test/data/query_p0/join/test_join.out
index c19bf29042..94d5e31dae 100644
--- a/regression-test/data/query_p0/join/test_join.out
+++ b/regression-test/data/query_p0/join/test_join.out
@@ -2714,3 +2714,83 @@ false true true false false
5 2.200 null \N 2019-09-09T00:00 8.9 2 \N 2 \N \N 8.9
5 2.200 null \N 2019-09-09T00:00 8.9 3 \N null 2019-09-09 \N 8.9
+-- !sql --
+\N
+1
+2
+3
+4
+5
+6
+7
+8
+9
+10
+11
+12
+13
+14
+15
+
+-- !sql --
+
+-- !sql --
+
+-- !sql --
+\N
+1
+2
+3
+4
+5
+6
+7
+8
+9
+10
+11
+12
+13
+14
+15
+
+-- !sql --
+\N
+1
+2
+3
+4
+5
+6
+7
+8
+9
+10
+11
+12
+13
+14
+15
+
+-- !sql --
+
+-- !sql --
+
+-- !sql --
+\N
+1
+2
+3
+4
+5
+6
+7
+8
+9
+10
+11
+12
+13
+14
+15
+
diff --git a/regression-test/suites/query_p0/join/test_join.groovy b/regression-test/suites/query_p0/join/test_join.groovy
index 146ab61de4..5137156f6a 100644
--- a/regression-test/suites/query_p0/join/test_join.groovy
+++ b/regression-test/suites/query_p0/join/test_join.groovy
@@ -1219,7 +1219,15 @@ suite("test_join", "query,p0") {
sql"""drop table ${table_3}"""
sql"""drop table ${table_4}"""
-
+ qt_sql """select k1 from baseall left semi join test on true order by k1;"""
+ qt_sql """select k1 from baseall left semi join test on false order by k1;"""
+ qt_sql """select k1 from baseall left anti join test on true order by k1;"""
+ qt_sql """select k1 from baseall left anti join test on false order by k1;"""
+
+ qt_sql """select k1 from test right semi join baseall on true order by k1;"""
+ qt_sql """select k1 from test right semi join baseall on false order by k1;"""
+ qt_sql """select k1 from test right anti join baseall on true order by k1;"""
+ qt_sql """select k1 from test right anti join baseall on false order by k1;"""
// test bucket shuffle join, github issue #6171
sql"""create database if not exists test_issue_6171"""
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org