Uh oh!
There was an error while loading. Please reload this page.
[enhance](nereids) add rewrite rule SplitJoinForNullSkew - #44357
[enhance](nereids) add rewrite rule SplitJoinForNullSkew#44357feiniaofeiafei wants to merge 6 commits into
Conversation
doris-robot
commented
Nov 20, 2024
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
3985e79 to
131b44aComparefeiniaofeiafei
commented
Nov 20, 2024
run buildall |
7c0ed99 to
82e7176Comparefeiniaofeiafei
commented
Nov 20, 2024
run buildall |
c96d557 to
1b40cc5Comparefeiniaofeiafei
commented
Nov 20, 2024
run buidall |
feiniaofeiafei
commented
Nov 21, 2024
run buildall |
feiniaofeiafei
commented
Nov 21, 2024
run p0 |
doris-robot
commented
Nov 21, 2024
TPC-H: Total hot run time: 39963 ms |
doris-robot
commented
Nov 21, 2024
TPC-DS: Total hot run time: 198243 ms |
doris-robot
commented
Nov 21, 2024
ClickBench: Total hot run time: 32.02 s |
| public Rule build() { | ||
| return logicalJoin(any(), any()) | ||
| .when(join -> join.getJoinType().isLeftJoin()) | ||
| .when(join -> join.getHashJoinConjuncts().size() == 1) |
There was a problem hiding this comment.
should check mark join conjuncts
| .when(join -> join.getJoinType().isLeftJoin()) | ||
| .when(join -> join.getHashJoinConjuncts().size() == 1) | ||
| .thenApply(ctx -> { | ||
| Set<Integer> enableNereidsRules = ctx.cascadesContext.getConnectContext() |
There was a problem hiding this comment.
do we need add this? enableRules removed in this PR: #44769
| @Override | ||
| public Rule build() { | ||
| return logicalJoin(any(), any()) | ||
| .when(join -> join.getJoinType().isLeftJoin()) |
There was a problem hiding this comment.
why not process right outer join?
| Plan deepCopyJoin = LogicalPlanDeepCopier.INSTANCE.deepCopy(newJoin, new DeepCopierContext()); | ||
| // avoid duplicate application of rules | ||
| if (left instanceof LogicalFilter) { |
There was a problem hiding this comment.
why not just check filter's conjuncts contain is not null?
There was a problem hiding this comment.
you are right, I make it too complicated.
| .toRule(RuleType.JOIN_SPLIT_FOR_NULL_SKEW); | ||
| } | ||
| private Plan splitJoin(LogicalJoin<Plan, Plan> join) { |
There was a problem hiding this comment.
should not rewrite if leftExpr is not null already
ae357a1 to
ff5921cComparefeiniaofeiafei
commented
Dec 2, 2024
run buildall |
ff5921c to
94c526aComparefeiniaofeiafei
commented
Dec 3, 2024
run buildall |
doris-robot
commented
Dec 3, 2024
TPC-H: Total hot run time: 39859 ms |
doris-robot
commented
Dec 3, 2024
TPC-DS: Total hot run time: 198205 ms |
doris-robot
commented
Dec 3, 2024
ClickBench: Total hot run time: 32.28 s |
b688cb2 to
9bcb488Comparefeiniaofeiafei
commented
Feb 14, 2025
run buildall |
doris-robot
commented
Feb 14, 2025
TPC-H: Total hot run time: 31583 ms |
doris-robot
commented
Feb 14, 2025
TPC-DS: Total hot run time: 185766 ms |
doris-robot
commented
Feb 14, 2025
ClickBench: Total hot run time: 30.46 s |
861da34 to
6bb80b0Comparefeiniaofeiafei
commented
Feb 17, 2025
run buildall |
doris-robot
commented
Feb 17, 2025
TPC-H: Total hot run time: 31676 ms |
doris-robot
commented
Feb 17, 2025
TPC-DS: Total hot run time: 190308 ms |
6bb80b0 to
5c1c87bComparedoris-robot
commented
Feb 17, 2025
ClickBench: Total hot run time: 30.13 s |
5c1c87b to
2b18302Comparefeiniaofeiafei
commented
Feb 17, 2025
run buildall |
doris-robot
commented
Feb 17, 2025
TPC-H: Total hot run time: 31933 ms |
doris-robot
commented
Feb 17, 2025
TPC-DS: Total hot run time: 184228 ms |
doris-robot
commented
Feb 17, 2025
ClickBench: Total hot run time: 30.22 s |
rule can execute Avoid duplicate application of rules add comment add test remove unrelated code add test modify code by comments fix compile
What problem does this PR solve?
add transform rule:
only support left join with only 1 hash join conjuncts , and without other join conjuncts.
Since there is sometimes null value skew on the join key, which can lead to prolonged execution times, the join is split into two parts based on whether the join key is null. This can accelerate the query.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)