morningman closed pull request #266: Fix equal conjuncts which refer more than
two tuples registered unsuccessfully
URL: https://github.com/apache/incubator-doris/pull/266
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/fe/src/main/java/org/apache/doris/analysis/Analyzer.java
b/fe/src/main/java/org/apache/doris/analysis/Analyzer.java
index d59e4e03..23ee0f0f 100644
--- a/fe/src/main/java/org/apache/doris/analysis/Analyzer.java
+++ b/fe/src/main/java/org/apache/doris/analysis/Analyzer.java
@@ -767,7 +767,6 @@ private void registerConjunct(Expr e) {
ArrayList<TupleId> tupleIds = Lists.newArrayList();
ArrayList<SlotId> slotIds = Lists.newArrayList();
e.getIds(tupleIds, slotIds);
-
// register full join conjuncts
registerFullOuterJoinedConjunct(e);
@@ -803,7 +802,7 @@ private void registerConjunct(Expr e) {
if (binaryPred.getOp() != BinaryPredicate.Operator.EQ) {
return;
}
- if (tupleIds.size() != 2) {
+ if (tupleIds.size() < 2) {
return;
}
@@ -1188,6 +1187,20 @@ public boolean isFullOuterJoined(Expr e) {
return globalState.fullOuterJoinedConjuncts.containsKey(e.getId());
}
+ public TableRef getOjRef(Expr e) {
+ return globalState.ojClauseByConjunct.get(e.getId());
+ }
+
+ /**
+ * Returns false if 'e' originates from an outer-join On-clause and it is
incorrect to
+ * evaluate 'e' at a node materializing 'tids'. Returns true otherwise.
+ */
+ public boolean canEvalOuterJoinedConjunct(Expr e, List<TupleId> tids) {
+ TableRef outerJoin = getOjRef(e);
+ if (outerJoin == null) return true;
+ return tids.containsAll(outerJoin.getAllTableRefIds());
+ }
+
/**
* Returns list of candidate equi-join conjuncts to be evaluated by the
join node
* that is specified by the table ref ids of its left and right children.
@@ -1195,27 +1208,59 @@ public boolean isFullOuterJoined(Expr e) {
* from its On-clause are returned. If an equi-join conjunct is full outer
joined,
* then it is only added to the result if this join is the one to
full-outer join it.
*/
- public List<Expr> getEqJoinConjuncts(TupleId id, TableRef rhsRef) {
- List<ExprId> conjunctIds = globalState.eqJoinConjuncts.get(id);
- if (conjunctIds == null) {
- return null;
+ public List<Expr> getEqJoinConjuncts(List<TupleId> lhsTblRefIds,
+ List<TupleId> rhsTblRefIds) {
+ // Contains all equi-join conjuncts that have one child fully bound by
one of the
+ // rhs table ref ids (the other child is not bound by that rhs table
ref id).
+ List<ExprId> conjunctIds = Lists.newArrayList();
+ for (TupleId rhsId: rhsTblRefIds) {
+ List<ExprId> cids = globalState.eqJoinConjuncts.get(rhsId);
+ if (cids == null) continue;
+ for (ExprId eid: cids) {
+ if (!conjunctIds.contains(eid)) conjunctIds.add(eid);
+ }
}
- List<Expr> result = Lists.newArrayList();
+
+ // Since we currently prevent join re-reordering across outer joins,
we can never
+ // have a bushy outer join with multiple rhs table ref ids. A busy
outer join can
+ // only be constructed with an inline view (which has a single table
ref id).
List<ExprId> ojClauseConjuncts = null;
- if (rhsRef != null) {
- Preconditions.checkState(rhsRef.getJoinOp().isOuterJoin());
- ojClauseConjuncts =
globalState.conjunctsByOjClause.get(rhsRef.getId());
+ if (rhsTblRefIds.size() == 1) {
+ ojClauseConjuncts =
globalState.conjunctsByOjClause.get(rhsTblRefIds.get(0));
}
- for (ExprId conjunctId : conjunctIds) {
+
+ // List of table ref ids that the join node will 'materialize'.
+ List<TupleId> nodeTblRefIds = Lists.newArrayList(lhsTblRefIds);
+ nodeTblRefIds.addAll(rhsTblRefIds);
+ List<Expr> result = Lists.newArrayList();
+ for (ExprId conjunctId: conjunctIds) {
Expr e = globalState.conjuncts.get(conjunctId);
Preconditions.checkState(e != null);
- if (ojClauseConjuncts != null) {
- if (ojClauseConjuncts.contains(conjunctId)) {
- result.add(e);
- }
- } else {
- result.add(e);
+ if (!canEvalFullOuterJoinedConjunct(e, nodeTblRefIds) ||
+ !canEvalAntiJoinedConjunct(e, nodeTblRefIds) ||
+ !canEvalOuterJoinedConjunct(e, nodeTblRefIds)) {
+ continue;
}
+
+ if (ojClauseConjuncts != null &&
!ojClauseConjuncts.contains(conjunctId)) continue;
+ result.add(e);
+ }
+ return result;
+ }
+
+ /**
+ * return equal conjuncts, used by OlapScanNode.normalizePredicate and
SelectStmt.reorderTable
+ */
+ public List<Expr> getEqJoinConjuncts(TupleId id) {
+ final List<ExprId> conjunctIds = globalState.eqJoinConjuncts.get(id);
+ if (conjunctIds == null) {
+ return Lists.newArrayList();
+ }
+ final List<Expr> result = Lists.newArrayList();
+ for (ExprId conjunctId : conjunctIds) {
+ final Expr e = globalState.conjuncts.get(conjunctId);
+ Preconditions.checkState(e != null);
+ result.add(e);
}
return result;
}
@@ -1224,7 +1269,7 @@ public boolean isFullOuterJoined(Expr e) {
* Returns list of candidate equi-join conjuncts excluding auxiliary
predicates
*/
public List<Expr> getEqJoinConjunctsExcludeAuxPredicates(TupleId id) {
- final List<Expr> candidateEqJoinPredicates = getEqJoinConjuncts(id,
null);
+ final List<Expr> candidateEqJoinPredicates = getEqJoinConjuncts(id);
final Iterator<Expr> iterator = candidateEqJoinPredicates.iterator();
while (iterator.hasNext()) {
final Expr expr = iterator.next();
diff --git a/fe/src/main/java/org/apache/doris/analysis/SelectStmt.java
b/fe/src/main/java/org/apache/doris/analysis/SelectStmt.java
index af4995ff..eb10ed61 100644
--- a/fe/src/main/java/org/apache/doris/analysis/SelectStmt.java
+++ b/fe/src/main/java/org/apache/doris/analysis/SelectStmt.java
@@ -567,7 +567,7 @@ protected boolean reorderTable(Analyzer analyzer, TableRef
firstRef)
while (i < fromClause_.size()) {
TableRef tblRef = fromClause_.get(i);
// get all equal
- List<Expr> eqJoinPredicates =
analyzer.getEqJoinConjuncts(tblRef.getId(), null);
+ List<Expr> eqJoinPredicates =
analyzer.getEqJoinConjuncts(tblRef.getId());
List<TupleId> tuple_list = Lists.newArrayList();
Expr.getIds(eqJoinPredicates, tuple_list, null);
for (TupleId tid : tuple_list) {
diff --git a/fe/src/main/java/org/apache/doris/planner/OlapScanNode.java
b/fe/src/main/java/org/apache/doris/planner/OlapScanNode.java
index 42fd43ff..578c3fc6 100644
--- a/fe/src/main/java/org/apache/doris/planner/OlapScanNode.java
+++ b/fe/src/main/java/org/apache/doris/planner/OlapScanNode.java
@@ -294,16 +294,14 @@ public int compare(MaterializedIndex index1,
MaterializedIndex index2)
private void normalizePredicate(Analyzer analyzer) throws UserException {
// 1. Get Columns which has eqJoin on it
- List<Expr> eqJoinPredicate = analyzer.getEqJoinConjuncts(desc.getId(),
null);
- if (null != eqJoinPredicate) {
+ List<Expr> eqJoinPredicate = analyzer.getEqJoinConjuncts(desc.getId());
+ for (Expr expr : eqJoinPredicate) {
for (SlotDescriptor slot : desc.getSlots()) {
- for (Expr expr : eqJoinPredicate) {
- for (int i = 0; i < 2; ++i) {
- if (expr.getChild(i).isBound(slot.getId())) {
- eqJoinColumns.add(slot.getColumn().getName());
- LOG.debug("Add eqJoinColumn: ColName=" +
slot.getColumn().getName());
- break;
- }
+ for (int i = 0; i < 2; i++) {
+ if (expr.getChild(i).isBound(slot.getId())) {
+ eqJoinColumns.add(slot.getColumn().getName());
+ LOG.debug("Add eqJoinColumn: ColName=" +
slot.getColumn().getName());
+ break;
}
}
}
diff --git a/fe/src/main/java/org/apache/doris/planner/SingleNodePlanner.java
b/fe/src/main/java/org/apache/doris/planner/SingleNodePlanner.java
index 245a8357..21c87db5 100644
--- a/fe/src/main/java/org/apache/doris/planner/SingleNodePlanner.java
+++ b/fe/src/main/java/org/apache/doris/planner/SingleNodePlanner.java
@@ -1170,25 +1170,18 @@ private PlanNode createScanNode(Analyzer analyzer,
TableRef tblRef)
* inner joins, but only from the JOIN clause Returns the conjuncts in
'joinConjuncts' (in which "<lhs> = <rhs>" is
* returned as Pair(<lhs>, <rhs>)) and also in their original form in
'joinPredicates'.
*/
- private void getHashLookupJoinConjuncts(Analyzer analyzer, List<TupleId>
lhsIds, TableRef rhs,
+ private void getHashLookupJoinConjuncts(Analyzer analyzer, PlanNode left,
PlanNode right,
List<Pair<Expr, Expr>>
joinConjuncts, List<Expr> joinPredicates,
- Reference<String> errMsg) {
+ Reference<String> errMsg,
JoinOperator op) {
joinConjuncts.clear();
joinPredicates.clear();
- TupleId rhsId = rhs.getId();
- // List<TupleId> rhsIds = rhs.getMaterializedTupleIds();
- List<TupleId> rhsIds = rhsId.asList();
+ final List<TupleId> lhsIds = left.getTblRefIds();
+ final List<TupleId> rhsIds = right.getTblRefIds();
List<Expr> candidates;
- if (rhs.getJoinOp().isOuterJoin()) {
- // TODO: create test for this
- Preconditions.checkState(rhs.getOnClause() != null);
- candidates = analyzer.getEqJoinConjuncts(rhsId, rhs);
- } else {
- candidates = analyzer.getEqJoinConjuncts(rhsId, null);
- }
+ candidates = analyzer.getEqJoinConjuncts(lhsIds, rhsIds);
if (candidates == null) {
- if (rhs.getJoinOp().isOuterJoin() ||
rhs.getJoinOp().isSemiAntiJoin()) {
- errMsg.setRef("non-equal " + rhs.getJoinOp().toString() + "
is not supported");
+ if (op.isOuterJoin() || op.isSemiAntiJoin()) {
+ errMsg.setRef("non-equal " + op.toString() + " is not
supported");
LOG.warn(errMsg);
}
LOG.info("no candidates for join.");
@@ -1244,8 +1237,8 @@ private PlanNode createJoinNode(Analyzer analyzer,
PlanNode outer, TableRef oute
Reference<String> errMsg = new Reference<String>();
// get eq join predicates for the TableRefs' ids (not the PlanNodes'
ids, which
// are materialized)
- getHashLookupJoinConjuncts(analyzer, outer.getTblRefIds(), innerRef,
eqJoinConjuncts,
- eqJoinPredicates, errMsg);
+ getHashLookupJoinConjuncts(analyzer, outer, inner, eqJoinConjuncts,
+ eqJoinPredicates, errMsg, innerRef.getJoinOp());
if (eqJoinPredicates.isEmpty()) {
// only inner join can change to cross join
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
