morrySnow commented on code in PR #34993:
URL: https://github.com/apache/doris/pull/34993#discussion_r1608146552
##########
regression-test/data/nereids_rules_p0/transposeJoin/transposeSemiJoinAgg.out:
##########
@@ -19,14 +19,14 @@ PhysicalResultSink
-- !grouping_positive_case --
PhysicalResultSink
---hashJoin[LEFT_SEMI_JOIN] hashCondition=((T3.a = T2.a)) otherCondition=()
-----hashAgg[GLOBAL]
-------hashAgg[LOCAL]
+--hashAgg[GLOBAL]
+----hashAgg[LOCAL]
+------hashJoin[LEFT_SEMI_JOIN] hashCondition=((T3.a = T2.a)) otherCondition=()
Review Comment:
join agg upside down is ok?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/ColumnPruningPostProcessor.java:
##########
@@ -41,66 +35,13 @@
public class ColumnPruningPostProcessor extends PlanPostProcessor {
Review Comment:
rename to remove useless project?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/PlanPostProcessor.java:
##########
@@ -19,12 +19,17 @@
import org.apache.doris.nereids.CascadesContext;
import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.trees.plans.physical.AbstractPhysicalPlan;
import org.apache.doris.nereids.trees.plans.visitor.DefaultPlanRewriter;
/**
* PlanPostprocessor: a PlanVisitor to rewrite PhysicalPlan to new
PhysicalPlan.
*/
public class PlanPostProcessor extends DefaultPlanRewriter<CascadesContext> {
+ @Override
+ public Plan visit(Plan plan, CascadesContext context) {
+ return ((AbstractPhysicalPlan) super.visit(plan,
context)).copyStatsAndGroupIdFrom((AbstractPhysicalPlan) plan);
Review Comment:
`copyStatsAndGroupIdFrom` only if `super.visit(plan, context)) != plan`?
##########
regression-test/data/nereids_hint_tpcds_p0/shape/query1.out:
##########
@@ -25,19 +25,21 @@ PhysicalCteAnchor ( cteId=CTEId#0 )
------------------hashAgg[LOCAL]
--------------------PhysicalDistribute[DistributionSpecExecutionAny]
----------------------PhysicalCteConsumer ( cteId=CTEId#0 )
---------------PhysicalProject
-----------------hashJoin[INNER_JOIN] hashCondition=((store.s_store_sk =
ctr1.ctr_store_sk)) otherCondition=() build RFs:RF1 s_store_sk->[ctr_store_sk]
-------------------PhysicalDistribute[DistributionSpecHash]
---------------------hashJoin[INNER_JOIN] hashCondition=((ctr1.ctr_customer_sk
= customer.c_customer_sk)) otherCondition=()
-----------------------PhysicalDistribute[DistributionSpecHash]
-------------------------PhysicalProject
---------------------------PhysicalOlapScan[customer]
-----------------------PhysicalDistribute[DistributionSpecHash]
-------------------------PhysicalCteConsumer ( cteId=CTEId#0 ) apply RFs: RF1
-------------------PhysicalDistribute[DistributionSpecHash]
---------------------PhysicalProject
-----------------------filter((store.s_state = 'TN'))
-------------------------PhysicalOlapScan[store]
+--------------PhysicalDistribute[DistributionSpecHash]
+----------------PhysicalProject
Review Comment:
join order changed? hint not take effect?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/TransposeAggSemiJoinProject.java:
##########
@@ -40,7 +43,8 @@ public Rule build() {
if (!TransposeSemiJoinAgg.canTranspose(agg, join)) {
return null;
}
- return
join.withChildren(agg.withChildren(project.withChildren(join.left())),
join.right());
+ Plan newJoin =
join.withChildren(agg.withChildren(project.withChildren(join.left())),
join.right());
+ return new LogicalProject<>(new
ArrayList<>(agg.getOutput()), newJoin);
Review Comment:
```suggestion
return new
LogicalProject<>(ImmutableList.copyOf(agg.getOutput()), newJoin);
```
##########
regression-test/data/nereids_tpcds_shape_sf1000_p0/bs_downgrade_shape/query13.out:
##########
@@ -5,10 +5,7 @@ PhysicalResultSink
----PhysicalDistribute[DistributionSpecGather]
------hashAgg[LOCAL]
--------PhysicalProject
-----------hashJoin[INNER_JOIN] hashCondition=((store.s_store_sk =
store_sales.ss_store_sk)) otherCondition=() build RFs:RF4
ss_store_sk->[s_store_sk]
-------------PhysicalDistribute[DistributionSpecHash]
---------------PhysicalProject
-----------------PhysicalOlapScan[store] apply RFs: RF4
+----------hashJoin[INNER_JOIN] hashCondition=((store.s_store_sk =
store_sales.ss_store_sk)) otherCondition=() build RFs:RF4
s_store_sk->[ss_store_sk]
Review Comment:
store from left side to right side?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/MergeProjectPostProcessor.java:
##########
@@ -33,16 +31,14 @@ public class MergeProjectPostProcessor extends
PlanPostProcessor {
@Override
public PhysicalProject visitPhysicalProject(PhysicalProject<? extends
Plan> project, CascadesContext ctx) {
+ project = (PhysicalProject<? extends Plan>) super.visit(project, ctx);
Plan child = project.child();
- Plan newChild = child.accept(this, ctx);
Review Comment:
why change this rule? what's the problem of it?
##########
regression-test/suites/query_p0/join/test_join.groovy:
##########
@@ -1266,6 +1266,7 @@ suite("test_join", "query,p0") {
DISTRIBUTED BY HASH(`ID`) BUCKETS 32
PROPERTIES("replication_num"="1");"""
}
+ sql "SET disable_join_reorder=true"
Review Comment:
set disable to false after explain check
--
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: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
