Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix equal conjuncts which refer more than two tuples registered unsuccessfully #266

Merged
merged 1 commit into from
Nov 1, 2018
Merged

Fix equal conjuncts which refer more than two tuples registered unsuccessfully #266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 63 additions & 18 deletions fe/src/main/java/org/apache/doris/analysis/Analyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,6 @@ private void registerConjunct(Expr e) {
ArrayList<TupleId> tupleIds = Lists.newArrayList();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit log format is wrong.

ArrayList<SlotId> slotIds = Lists.newArrayList();
e.getIds(tupleIds, slotIds);

// register full join conjuncts
registerFullOuterJoinedConjunct(e);

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -1188,34 +1187,80 @@ 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.
* If the join to be performed is an outer join, then only equi-join conjuncts
* 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;
}
Expand All @@ -1224,7 +1269,7 @@ public List<Expr> getEqJoinConjuncts(TupleId id, TableRef rhsRef) {
* 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();
Expand Down
2 changes: 1 addition & 1 deletion fe/src/main/java/org/apache/doris/analysis/SelectStmt.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
16 changes: 7 additions & 9 deletions fe/src/main/java/org/apache/doris/planner/OlapScanNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
Expand Down
25 changes: 9 additions & 16 deletions fe/src/main/java/org/apache/doris/planner/SingleNodePlanner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand Down Expand Up @@ -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
Expand Down