github-actions[bot] commented on code in PR #65467:
URL: https://github.com/apache/doris/pull/65467#discussion_r3567835847
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java:
##########
@@ -524,23 +560,40 @@ private LogicalPlan makeJoinPlan(LogicalPlan leftChild,
LogicalPlan rightChild,
if (!distributeJoinType.equals("join")) {
distributeHint = strToHint.get(distributeJoinType);
}
- List<Expression> conditions = getJoinConditions(
+ // Collect candidate conditions whose bitmap is covered by this join.
+ // Each entry carries the original join type from
CollectJoinConstraint.
+ List<FilterEntry> candidateEntries = collectJoinConditions(
getFilters(), leftChild, rightChild);
- Pair<List<Expression>, List<Expression>> pair =
JoinUtils.extractExpressionForHashTable(
- leftChild.getOutput(), rightChild.getOutput(), conditions);
- // leading hint would set status inside if not success
+ List<Expression> candidateExprs = new ArrayList<>();
+ for (FilterEntry e : candidateEntries) {
+ candidateExprs.add(e.expr);
+ }
+ // Determine join type using all candidate expressions for constraint
matching.
JoinType joinType = computeJoinType(getBitmap(leftChild),
- getBitmap(rightChild), conditions);
+ getBitmap(rightChild), candidateExprs);
if (joinType == null) {
+ // Put back all candidates since we cannot build this join.
+ filters.addAll(candidateEntries);
this.setStatus(HintStatus.SYNTAX_ERROR);
this.setErrorMessage("JoinType can not be null");
- } else if (!isConditionJoinTypeMatched(conditions, joinType)) {
- this.setStatus(HintStatus.UNUSED);
- this.setErrorMessage("condition does not matched joinType");
+ return null;
+ }
+ // Keep only entries whose original type is compatible with the
generated
+ // join type. Incompatible entries are returned to the global filter
list
+ // so they can be consumed at a later (higher) join level.
+ List<Expression> conditions = new ArrayList<>();
+ for (FilterEntry entry : candidateEntries) {
+ if (isJoinTypeCompatible(entry.originalType, joinType)) {
+ conditions.add(entry.expr);
+ } else {
+ filters.add(entry);
Review Comment:
This put-back path can still finish with a successful leading hint after
dropping a required predicate. For example, with `(a FULL OUTER JOIN b ON a.k =
b.k) JOIN c` and `leading(a c b)`, the full-outer predicate is not covered by
the lower `{a,c}` join. At the final join, the full-outer constraint does not
match because the generated children are `{a,c}` and `{b}`, so
`computeJoinType()` sees the candidate expression and returns `INNER_JOIN`. The
compatibility loop then rejects the `FULL_OUTER_JOIN` entry here and puts it
back, but there is no later scan/join to consume it. The only leftover check in
`generateLeadingJoinPlan()` is behind `Utils.enableAssert`, which is false in a
normal JVM, and the method then marks the hint `SUCCESS`; the generated plan
has lost `a.k = b.k` and no longer preserves the full outer join semantics.
Please make leftover returned filters fail the hint unconditionally, and
compute `INNER` vs `CROSS` from the compatibility-filtered conditions so rejecte
d candidates cannot shape the generated join.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
