-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[fix](nereids) fix execute err which throw eq function not exist exception when join reorder #54953
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.
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
[fix](nereids) fix execute err which throw eq function not exist exception when join reorder #54953
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
TPC-H: Total hot run time: 33609 ms |
TPC-DS: Total hot run time: 184376 ms |
ClickBench: Total hot run time: 32.97 s |
| } | ||
|
|
||
| return newJoin; | ||
| return JoinUtils.adjustJoinConjunctsNullable(newJoin); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should only process if the subplan contains outer join?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have fixed
| Map<Slot, Slot> childSlotMap = new HashMap<>(); | ||
| for (Plan child : join.children()) { | ||
| for (Slot slot : child.getOutput()) { | ||
| childSlotMap.put(slot, slot); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should not replace directly, generate a new slot with correct nullable is better, could reuse or refer to fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AdjustNullable.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have fixed
|
run buildall |
FE UT Coverage ReportIncrement line coverage |
TPC-H: Total hot run time: 34571 ms |
TPC-DS: Total hot run time: 184971 ms |
ClickBench: Total hot run time: 33.39 s |
|
run buildall |
TPC-H: Total hot run time: 34147 ms |
TPC-DS: Total hot run time: 184779 ms |
ClickBench: Total hot run time: 32.44 s |
| } | ||
|
|
||
| if (newJoin.getJoinType().isOuterJoin()) { | ||
| return JoinUtils.adjustJoinConjunctsNullable(newJoin); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let newJoin = JoinUtils.adjustJoinConjunctsNullable(newJoin) ?
| /** | ||
| * Adjust nullable attribute of slot reference in expression. | ||
| */ | ||
| public static Expression doUpdateExpression(AtomicBoolean changed, Expression input, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if doUpdateExpression make public, maybe we can remove the arg changed and return Optinal to indicate whether have changed ? what's more, combine the arg debugCheck and isAnalyzedPhase into one: needCheck, then when AdjustNullable call doUpdateExpression, needCheck = debugCheck && !isAnalyzedPhase, when JsonUtils call doUpdateExpression , let needCheck = false ?
| * if the join change to '(b left join c) right a where b.condition > 1', | ||
| * the nullable property of b.condition should be false | ||
| */ | ||
| public static LogicalJoin<Plan, Plan> adjustJoinConjunctsNullable(LogicalJoin<Plan, Plan> join) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the logical should be like AdjustNullable.visitLogicalJoin, but here forget to update mark join conditions. maybe can reconstruct AdjustNullable.visitLogicalJoin, and expose a public function AdjustNullable.adjustJoinNullableWithReplaceSlots(join, replaceMap)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have fixed
…ption when join reorder
…ption when join reorder
49aa1e7 to
11084de
Compare
|
run buildall |
TPC-H: Total hot run time: 34062 ms |
TPC-DS: Total hot run time: 184650 ms |
|
run buildall |
TPC-H: Total hot run time: 34227 ms |
TPC-DS: Total hot run time: 187574 ms |
ClickBench: Total hot run time: 32.79 s |
yujun777
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
PR approved by anyone and no changes requested. |
|
PR approved by at least one committer and no changes requested. |
…ption when join reorder (#54953) ### What problem does this PR solve? if query as following: select xx from a left join b on a.id = b.id left join c on b.id = c.id the column in table a, b, c are all non nullable. after join reorder the query is as following: select xx from b left join c on b.id = c.id right join a on on a.id = b.id the the nullable condition in join conjuncts maybe nullable wrongly. this would cause be exec fail with err msg `[INTERNAL_ERROR]Could not find function eq, arg String return Nullable(BOOL) ` the pr fix this
…ption when join reorder (apache#54953) ### What problem does this PR solve? if query as following: select xx from a left join b on a.id = b.id left join c on b.id = c.id the column in table a, b, c are all non nullable. after join reorder the query is as following: select xx from b left join c on b.id = c.id right join a on on a.id = b.id the the nullable condition in join conjuncts maybe nullable wrongly. this would cause be exec fail with err msg `[INTERNAL_ERROR]Could not find function eq, arg String return Nullable(BOOL) ` the pr fix this
…t exist exception when join reorder apache#54953 (apache#5643) commitId: selectdb/selectdb-core@fb26e43 pr: apache#54953
What problem does this PR solve?
if query as following:
the column in table a, b, c are all non nullable.
after join reorder the query is as following:
the the nullable condition in join conjuncts maybe nullable wrongly.
this would cause be exec fail with err msg
[INTERNAL_ERROR]Could not find function eq, arg String return Nullable(BOOL)the pr fix this
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)