From 70e5681d7a125aeaf75ed0f8914ec06f7ae5cb4c Mon Sep 17 00:00:00 2001 From: englefly Date: Wed, 20 Jul 2022 11:50:21 +0800 Subject: [PATCH 1/2] [refactor] rename GroupExpression.getParent() to getOwnerGroup() --- .../nereids/jobs/cascades/ApplyRuleJob.java | 2 +- .../jobs/cascades/CostAndEnforcerJob.java | 6 +++--- .../org/apache/doris/nereids/memo/Group.java | 8 ++++---- .../doris/nereids/memo/GroupExpression.java | 10 +++++----- .../org/apache/doris/nereids/memo/Memo.java | 20 +++++++++---------- .../pattern/GroupExpressionMatching.java | 2 +- .../EnforceMissingPropertiesHelper.java | 8 ++++---- 7 files changed, 28 insertions(+), 28 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/cascades/ApplyRuleJob.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/cascades/ApplyRuleJob.java index cdf97c2100d880..823da86d8ec28d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/cascades/ApplyRuleJob.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/cascades/ApplyRuleJob.java @@ -63,7 +63,7 @@ public void execute() throws AnalysisException { List newPlans = rule.transform(plan, context.getPlannerContext()); for (Plan newPlan : newPlans) { GroupExpression newGroupExpression = context.getPlannerContext().getMemo() - .copyIn(newPlan, groupExpression.getParent(), rule.isRewrite()); + .copyIn(newPlan, groupExpression.getOwnerGroup(), rule.isRewrite()); if (newPlan instanceof LogicalPlan) { pushTask(new DeriveStatsJob(newGroupExpression, context)); if (exploredOnly) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/cascades/CostAndEnforcerJob.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/cascades/CostAndEnforcerJob.java index 72a53cbcf0b34b..520f219522568e 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/cascades/CostAndEnforcerJob.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/cascades/CostAndEnforcerJob.java @@ -162,7 +162,7 @@ public void execute1() { } PlanContext planContext = new PlanContext(groupExpression); // TODO: calculate stats. - groupExpression.getParent().setStatistics(planContext.getStatistics()); + groupExpression.getOwnerGroup().setStatistics(planContext.getStatistics()); enforce(outputProperty, childrenInputProperties); } @@ -193,7 +193,7 @@ private void enforce(PhysicalProperties outputProperty, List // enforcedProperty is superset of requiredProperty if (!addEnforcedProperty.equals(requiredProperties)) { - putProperty(groupExpression.getParent().getBestExpression(addEnforcedProperty), + putProperty(groupExpression.getOwnerGroup().getBestExpression(addEnforcedProperty), requiredProperties, requiredProperties, Lists.newArrayList(outputProperty)); } } else { @@ -217,7 +217,7 @@ private void putProperty(GroupExpression groupExpression, // and shuffle join two types outputProperty. groupExpression.putOutputPropertiesMap(outputProperty, requiredProperty); } - this.groupExpression.getParent().setBestPlan(groupExpression, + this.groupExpression.getOwnerGroup().setBestPlan(groupExpression, curTotalCost, requiredProperty); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Group.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Group.java index fbb9a862c705e2..aa681c1ac00fb5 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Group.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Group.java @@ -66,7 +66,7 @@ public Group(GroupId groupId, GroupExpression groupExpression, LogicalProperties this.physicalExpressions.add(groupExpression); } this.logicalProperties = logicalProperties; - groupExpression.setParent(this); + groupExpression.setOwnerGroup(this); } public GroupId getGroupId() { @@ -93,7 +93,7 @@ public GroupExpression addGroupExpression(GroupExpression groupExpression) { } else { physicalExpressions.add(groupExpression); } - groupExpression.setParent(this); + groupExpression.setOwnerGroup(this); return groupExpression; } @@ -109,7 +109,7 @@ public GroupExpression removeGroupExpression(GroupExpression groupExpression) { } else { physicalExpressions.remove(groupExpression); } - groupExpression.setParent(null); + groupExpression.setOwnerGroup(null); return groupExpression; } @@ -121,7 +121,7 @@ public GroupExpression removeGroupExpression(GroupExpression groupExpression) { */ public GroupExpression rewriteLogicalExpression(GroupExpression newExpression, LogicalProperties logicalProperties) { - newExpression.setParent(this); + newExpression.setOwnerGroup(this); this.logicalProperties = logicalProperties; GroupExpression oldExpression = getLogicalExpression(); logicalExpressions.clear(); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/GroupExpression.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/GroupExpression.java index 3ff6d9b4dc5c62..6a7a6c78eab745 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/GroupExpression.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/GroupExpression.java @@ -36,7 +36,7 @@ * Representation for group expression in cascades optimizer. */ public class GroupExpression { - private Group parent; + private Group ownerGroup; private List children; private final Plan plan; private final BitSet ruleMasks; @@ -81,12 +81,12 @@ public void addChild(Group child) { children.add(child); } - public Group getParent() { - return parent; + public Group getOwnerGroup() { + return ownerGroup; } - public void setParent(Group parent) { - this.parent = parent; + public void setOwnerGroup(Group ownerGroup) { + this.ownerGroup = ownerGroup; } public Plan getPlan() { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java index aac059f839fdd1..89c365c4219b4f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java @@ -47,7 +47,7 @@ public class Memo { private Group root; public Memo(Plan plan) { - root = copyIn(plan, null, false).getParent(); + root = copyIn(plan, null, false).getOwnerGroup(); } public Group getRoot() { @@ -82,9 +82,9 @@ public GroupExpression copyIn(Plan node, @Nullable Group target, boolean rewrite if (child instanceof GroupPlan) { childrenGroups.add(((GroupPlan) child).getGroup()); } else if (child.getGroupExpression().isPresent()) { - childrenGroups.add(child.getGroupExpression().get().getParent()); + childrenGroups.add(child.getGroupExpression().get().getOwnerGroup()); } else { - childrenGroups.add(copyIn(child, null, rewrite).getParent()); + childrenGroups.add(copyIn(child, null, rewrite).getOwnerGroup()); } } node = replaceChildrenToGroupPlan(node, childrenGroups); @@ -133,9 +133,9 @@ private GroupExpression insertOrRewriteGroupExpression(GroupExpression groupExpr boolean rewrite, LogicalProperties logicalProperties) { GroupExpression existedGroupExpression = groupExpressions.get(groupExpression); if (existedGroupExpression != null - && existedGroupExpression.getParent().getLogicalProperties().equals(logicalProperties)) { - if (target != null && !target.getGroupId().equals(existedGroupExpression.getParent().getGroupId())) { - mergeGroup(target, existedGroupExpression.getParent()); + && existedGroupExpression.getOwnerGroup().getLogicalProperties().equals(logicalProperties)) { + if (target != null && !target.getGroupId().equals(existedGroupExpression.getOwnerGroup().getGroupId())) { + mergeGroup(target, existedGroupExpression.getOwnerGroup()); } return existedGroupExpression; } @@ -172,7 +172,7 @@ private void mergeGroup(Group source, Group destination) { List needReplaceChild = Lists.newArrayList(); groupExpressions.values().forEach(groupExpression -> { if (groupExpression.children().contains(source)) { - if (groupExpression.getParent().equals(destination)) { + if (groupExpression.getOwnerGroup().equals(destination)) { // cycle, we should not merge return; } @@ -189,8 +189,8 @@ private void mergeGroup(Group source, Group destination) { } } if (groupExpressions.containsKey(groupExpression)) { - // TODO: need to merge group recursively - groupExpression.getParent().removeGroupExpression(groupExpression); + GroupExpression that = groupExpressions.get(groupExpression); + mergeGroup(groupExpression.getOwnerGroup(), that.getOwnerGroup()); } else { groupExpressions.put(groupExpression, groupExpression); } @@ -210,7 +210,7 @@ private void mergeGroup(Group source, Group destination) { * Add enforcer expression into the target group. */ public void addEnforcerPlan(GroupExpression groupExpression, Group group) { - groupExpression.setParent(group); + groupExpression.setOwnerGroup(group); } private Plan replaceChildrenToGroupPlan(Plan plan, List childrenGroups) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/pattern/GroupExpressionMatching.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/pattern/GroupExpressionMatching.java index ec5ca0ff6a4d0b..0309859404b4d8 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/pattern/GroupExpressionMatching.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/pattern/GroupExpressionMatching.java @@ -159,7 +159,7 @@ private void assembleAllCombinationPlanTree(Plan root, Pattern rootPattern children.add(childrenPlans.get(i).get(childrenPlanIndex[i])); } - LogicalProperties logicalProperties = groupExpression.getParent().getLogicalProperties(); + LogicalProperties logicalProperties = groupExpression.getOwnerGroup().getLogicalProperties(); // assemble children: replace GroupPlan to real plan, // withChildren will erase groupExpression, so we must // withGroupExpression too. diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/EnforceMissingPropertiesHelper.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/EnforceMissingPropertiesHelper.java index aaa7a0d113f9be..54d26f5ca6954b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/EnforceMissingPropertiesHelper.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/EnforceMissingPropertiesHelper.java @@ -77,7 +77,7 @@ private PhysicalProperties enforceSort(PhysicalProperties oldOutputProperty) { oldOutputProperty.getOrderSpec()); newOutputProperty.setOrderSpec(context.getRequiredProperties().getOrderSpec()); GroupExpression enforcer = - context.getRequiredProperties().getOrderSpec().addEnforcer(groupExpression.getParent()); + context.getRequiredProperties().getOrderSpec().addEnforcer(groupExpression.getOwnerGroup()); updateCostWithEnforcer(enforcer, oldOutputProperty, newOutputProperty); @@ -89,7 +89,7 @@ private PhysicalProperties enforceDistribution(PhysicalProperties oldOutputPrope oldOutputProperty.getOrderSpec()); newOutputProperty.setDistributionSpec(context.getRequiredProperties().getDistributionSpec()); GroupExpression enforcer = - context.getRequiredProperties().getDistributionSpec().addEnforcer(groupExpression.getParent()); + context.getRequiredProperties().getDistributionSpec().addEnforcer(groupExpression.getOwnerGroup()); updateCostWithEnforcer(enforcer, oldOutputProperty, newOutputProperty); @@ -99,13 +99,13 @@ private PhysicalProperties enforceDistribution(PhysicalProperties oldOutputPrope private void updateCostWithEnforcer(GroupExpression enforcer, PhysicalProperties oldOutputProperty, PhysicalProperties newOutputProperty) { - context.getPlannerContext().getMemo().addEnforcerPlan(enforcer, groupExpression.getParent()); + context.getPlannerContext().getMemo().addEnforcerPlan(enforcer, groupExpression.getOwnerGroup()); curTotalCost += CostCalculator.calculateCost(enforcer); if (enforcer.updateLowestCostTable(newOutputProperty, Lists.newArrayList(oldOutputProperty), curTotalCost)) { enforcer.putOutputPropertiesMap(newOutputProperty, newOutputProperty); } - groupExpression.getParent().setBestPlan(enforcer, curTotalCost, newOutputProperty); + groupExpression.getOwnerGroup().setBestPlan(enforcer, curTotalCost, newOutputProperty); } private PhysicalProperties enforceSortAndDistribution(PhysicalProperties outputProperty, From d9666e28d6e07ff60ec4eca4b1c39094a3517e3d Mon Sep 17 00:00:00 2001 From: englefly Date: Wed, 20 Jul 2022 12:00:59 +0800 Subject: [PATCH 2/2] update memo.java --- .../src/main/java/org/apache/doris/nereids/memo/Memo.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java index 89c365c4219b4f..55a8e5ebcd038a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java @@ -189,8 +189,8 @@ private void mergeGroup(Group source, Group destination) { } } if (groupExpressions.containsKey(groupExpression)) { - GroupExpression that = groupExpressions.get(groupExpression); - mergeGroup(groupExpression.getOwnerGroup(), that.getOwnerGroup()); + // TODO: need to merge group recursively + groupExpression.getOwnerGroup().removeGroupExpression(groupExpression); } else { groupExpressions.put(groupExpression, groupExpression); }