morrySnow commented on code in PR #12439:
URL: https://github.com/apache/doris/pull/12439#discussion_r964835373
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/JoinReorderCommon.java:
##########
@@ -17,32 +17,23 @@
package org.apache.doris.nereids.rules.exploration.join;
-import org.apache.doris.nereids.trees.plans.GroupPlan;
-import org.apache.doris.nereids.trees.plans.logical.LogicalJoin;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
-/**
- * Common function for JoinCommute
- */
-public class JoinCommuteHelper {
+import java.util.List;
+import java.util.Optional;
- enum SwapType {
- BOTTOM_JOIN, ZIG_ZAG, ALL
+class JoinReorderCommon {
+ public enum Type {
+ INNER, OUTER
Review Comment:
```suggestion
INNER,
OUTER
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/JoinLAsscom.java:
##########
@@ -39,18 +63,13 @@ public class JoinLAsscom extends OneExplorationRuleFactory {
@Override
public Rule build() {
return logicalJoin(logicalJoin(), group())
- .when(JoinLAsscomHelper::check)
- .when(join -> join.getJoinType().isInnerJoin() ||
join.getJoinType().isLeftOuterJoin()
- && (join.left().getJoinType().isInnerJoin() ||
join.left().getJoinType().isLeftOuterJoin()))
- .then(topJoin -> {
-
- LogicalJoin<GroupPlan, GroupPlan> bottomJoin = topJoin.left();
- JoinLAsscomHelper helper = JoinLAsscomHelper.of(topJoin,
bottomJoin);
- if (!helper.initJoinOnCondition()) {
- return null;
- }
-
- return helper.newTopJoin();
- }).toRule(RuleType.LOGICAL_JOIN_L_ASSCOM);
+ .when(topJoin -> JoinLAsscomHelper.check(type, topJoin,
topJoin.left())).when(typeChecker)
Review Comment:
```suggestion
.when(topJoin -> JoinLAsscomHelper.check(type, topJoin,
topJoin.left()))
.when(typeChecker)
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/JoinReorderContext.java:
##########
@@ -38,55 +39,71 @@ public class JoinReorderContext {
public JoinReorderContext() {
}
+ /**
+ * copy a JoinReorderContext.
+ */
public void copyFrom(JoinReorderContext joinReorderContext) {
this.hasCommute = joinReorderContext.hasCommute;
+ this.hasLAsscom = joinReorderContext.hasLAsscom;
this.hasExchange = joinReorderContext.hasExchange;
this.hasLeftAssociate = joinReorderContext.hasLeftAssociate;
this.hasRightAssociate = joinReorderContext.hasRightAssociate;
this.hasCommuteZigZag = joinReorderContext.hasCommuteZigZag;
}
+ /**
+ * clear all.
+ */
public void clear() {
hasCommute = false;
+ hasLAsscom = false;
hasCommuteZigZag = false;
hasExchange = false;
hasRightAssociate = false;
hasLeftAssociate = false;
}
- public boolean hasCommute() {
+ public boolean isHasCommute() {
return hasCommute;
}
public void setHasCommute(boolean hasCommute) {
this.hasCommute = hasCommute;
}
- public boolean hasExchange() {
+ public boolean isHasLAsscom() {
+ return hasLAsscom;
+ }
+
+ public void setHasLAsscom(boolean hasLAsscom) {
+ this.hasLAsscom = hasLAsscom;
+ }
+
+ public boolean isHasExchange() {
return hasExchange;
}
public void setHasExchange(boolean hasExchange) {
this.hasExchange = hasExchange;
}
- public boolean hasRightAssociate() {
+ public boolean isHasRightAssociate() {
Review Comment:
old name is better
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleSet.java:
##########
@@ -62,6 +66,39 @@ public class RuleSet {
.add(new LogicalAssertNumRowsToPhysicalAssertNumRows())
.build();
+ public static final List<Rule> LEFT_DEEP_TREE_JOIN_REORDER =
planRuleFactories()
+ .add(JoinCommute.OUTER_LEFT_DEEP)
+ .add(JoinLAsscom.INNER)
+ .add(JoinLAsscomProject.INNER)
+ .add(JoinLAsscom.OUTER)
+ .add(JoinLAsscomProject.OUTER)
+ // semi join Transpose ....
+ .build();
+
+ public static final List<Rule> ZIG_ZAG_TREE_JOIN_REORDER =
planRuleFactories()
+ .add(JoinCommute.OUTER_ZIG_ZAG)
+ .add(JoinLAsscom.INNER)
+ .add(JoinLAsscomProject.INNER)
+ .add(JoinLAsscom.OUTER)
+ .add(JoinLAsscomProject.OUTER)
+ // semi join Transpose ....
+ .build();
+
+ public static final List<Rule> BUSHY_TREE_JOIN_REORDER =
planRuleFactories()
+ .add(JoinCommute.OUTER_BUSHY)
+ // .add(JoinLeftAssociate.INNER)
Review Comment:
add TODO to explain what to do next step
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/JoinCommute.java:
##########
@@ -17,54 +17,69 @@
package org.apache.doris.nereids.rules.exploration.join;
-import org.apache.doris.nereids.annotation.Developing;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.rules.exploration.OneExplorationRuleFactory;
-import
org.apache.doris.nereids.rules.exploration.join.JoinCommuteHelper.SwapType;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
import org.apache.doris.nereids.trees.plans.GroupPlan;
import org.apache.doris.nereids.trees.plans.logical.LogicalJoin;
+import org.apache.doris.nereids.util.Utils;
+
+import java.util.ArrayList;
+import java.util.List;
/**
* Join Commute
*/
-@Developing
public class JoinCommute extends OneExplorationRuleFactory {
- public static final JoinCommute SWAP_OUTER_COMMUTE_BOTTOM_JOIN = new
JoinCommute(true, SwapType.BOTTOM_JOIN);
- public static final JoinCommute SWAP_OUTER_SWAP_ZIG_ZAG = new
JoinCommute(true, SwapType.ZIG_ZAG);
+ public static final JoinCommute OUTER_LEFT_DEEP = new
JoinCommute(SwapType.LEFT_DEEP);
+ public static final JoinCommute OUTER_ZIG_ZAG = new
JoinCommute(SwapType.ZIG_ZAG);
+ public static final JoinCommute OUTER_BUSHY = new
JoinCommute(SwapType.BUSHY);
- private final boolean swapOuter;
private final SwapType swapType;
- public JoinCommute(boolean swapOuter) {
- this.swapOuter = swapOuter;
- this.swapType = SwapType.ALL;
+ public JoinCommute(SwapType swapType) {
+ this.swapType = swapType;
}
- public JoinCommute(boolean swapOuter, SwapType swapType) {
- this.swapOuter = swapOuter;
- this.swapType = swapType;
+ enum SwapType {
+ LEFT_DEEP, ZIG_ZAG, BUSHY
}
@Override
public Rule build() {
- return innerLogicalJoin().when(JoinCommuteHelper::check).then(join -> {
- // TODO: add project for mapping column output.
- // List<NamedExpression> newOutput = new
ArrayList<>(join.getOutput());
+ return innerLogicalJoin().when(this::check).then(join -> {
LogicalJoin<GroupPlan, GroupPlan> newJoin = new LogicalJoin<>(
join.getJoinType(),
join.getHashJoinConjuncts(),
join.getOtherJoinCondition(),
join.right(), join.left(),
join.getJoinReorderContext());
newJoin.getJoinReorderContext().setHasCommute(true);
- // if (swapType == SwapType.ZIG_ZAG && !isBottomJoin(join)) {
- // newJoin.getJoinReorderContext().setHasCommuteZigZag(true);
- // }
+ if (swapType == SwapType.ZIG_ZAG && isNotBottomJoin(join)) {
+ newJoin.getJoinReorderContext().setHasCommuteZigZag(true);
+ }
- // LogicalProject<LogicalJoin> project = new
LogicalProject<>(newOutput, newJoin);
- return newJoin;
+ return JoinReorderCommon.project(new
ArrayList<>(join.getOutput()), newJoin).get();
}).toRule(RuleType.LOGICAL_JOIN_COMMUTATIVE);
}
+
+ private boolean check(LogicalJoin<GroupPlan, GroupPlan> join) {
+ if (swapType == SwapType.LEFT_DEEP && isNotBottomJoin(join)) {
+ return false;
+ }
+
+ return !join.getJoinReorderContext().isHasCommute() &&
!join.getJoinReorderContext().isHasExchange();
+ }
+
+ private boolean isNotBottomJoin(LogicalJoin<GroupPlan, GroupPlan> join) {
+ // TODO: tmp way to judge bottomJoin
+ return containJoin(join.left()) || containJoin(join.right());
+ }
+
+ private boolean containJoin(GroupPlan groupPlan) {
+ List<SlotReference> output = Utils.getOutputSlotReference(groupPlan);
+ return
!output.stream().map(SlotReference::getQualifier).allMatch(output.get(0).getQualifier()::equals);
Review Comment:
i think this very inaccurate.
BTW, 'TODO tmp way to judge bottomJoin' should add to this function
--
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]
