-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[fix](Nereids): just pull up alias project above join through topn #32305
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
Conversation
|
Thank you for your contribution to Apache Doris. |
|
run buildall |
TPC-H: Total hot run time: 38028 ms |
11cc83d to
21d8b8e
Compare
|
run buildall |
|
run buildall |
| @Override | ||
| public Rule build() { | ||
| return logicalLimit(logicalProject().whenNot(p -> p.isAllSlots())) | ||
| return logicalLimit(logicalProject(logicalJoin().when(j -> j.getJoinType().isLeftRightOuterOrCrossJoin())) |
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.
Does it reduce the scope of this rule if adding the join pattern?
|
|
||
| class PullUpProjectUnderTopNTest implements MemoPatternMatchSupported { | ||
| private final LogicalOlapScan scan1 = PlanConstructor.newLogicalOlapScan(0, "t1", 0); | ||
| private final LogicalOlapScan scan2 = PlanConstructor.newLogicalOlapScan(1, "t2", 0); |
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.
why not add a new ut to cover the join with select alias, avoid overwrite the original one?
| new Cast(scan1.getOutput().get(1), VarcharType.SYSTEM_DEFAULT).alias("cast") | ||
| ); | ||
| LogicalPlan limit = new LogicalPlanBuilder(scan1) | ||
| .join(scan2, JoinType.LEFT_OUTER_JOIN, ImmutableList.of()) |
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.
Do we need to cover right outer and cross? since the code handle these two kinds.
| return logicalTopN(logicalProject().whenNot(p -> p.isAllSlots())) | ||
| .whenNot(topN -> topN.child().hasPushedDownToProjectionFunctions()) | ||
| return logicalTopN( | ||
| logicalProject(logicalJoin().when(j -> j.getJoinType().isLeftRightOuterOrCrossJoin())) |
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.
similar question, will it reduce this rule's scope if adding the join pattern judge?
|
run buildall |
|
PR approved by anyone and no changes requested. |
|
run buildall |
|
run buildall |
TPC-H: Total hot run time: 37532 ms |
TPC-DS: Total hot run time: 181258 ms |
|
run p0 |
|
run buildall |
TPC-H: Total hot run time: 37405 ms |
TPC-DS: Total hot run time: 180783 ms |
|
Load test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
|
run buildall |
TPC-H: Total hot run time: 37405 ms |
TPC-DS: Total hot run time: 180711 ms |
|
Load test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
|
PR approved by at least one committer and no changes requested. |
* [enhancement](Nereids): support more condition Date/DateTime Literal (apache#31858) (cherry picked from commit dbb7573) * [fix](Nereids): slot set in condition can be empty (apache#32169) (cherry picked from commit ca09213)
Proposed changes
Further comments
If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...