This is an automated email from the ASF dual-hosted git repository.
huajianlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new d663497230 [refactor] (Nereids) Memo.copyIn() return a pair<newly,
GroupExpression> (#10891)
d663497230 is described below
commit d6634972307d7d08797ed1b93814cb6b397d6518
Author: minghong <engle...@gmail.com>
AuthorDate: Wed Jul 27 10:33:25 2022 +0800
[refactor] (Nereids) Memo.copyIn() return a pair<newly, GroupExpression>
(#10891)
When we copy a plan into memo, we will check if this plan is already in
memo or it is a new plan.
In the new version of Memo.copyIn(), we encapsulate is_new and the plan's
corresponding group-expression.
The is_new is used to avoid repeatedly apply rules against the same plan,
and hence save optimize efforts.
Describe the overview of changes.
change Memo.copyIn() and related function interfaces
every time Memo.copyIn() is invoked, we check if the plan is already
recorded by memo or not. if plan is not new, we do not put its group into stack
for further optimization.
---
.../doris/nereids/jobs/cascades/ApplyRuleJob.java | 8 +++++++-
.../nereids/jobs/rewrite/RewriteBottomUpJob.java | 10 +++++----
.../nereids/jobs/rewrite/RewriteTopDownJob.java | 16 +++++++++++----
.../java/org/apache/doris/nereids/memo/Memo.java | 24 +++++++++++++---------
4 files changed, 39 insertions(+), 19 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 823da86d8e..781af4aa0d 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
@@ -17,6 +17,7 @@
package org.apache.doris.nereids.jobs.cascades;
+import org.apache.doris.common.Pair;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.jobs.Job;
import org.apache.doris.nereids.jobs.JobContext;
@@ -62,8 +63,13 @@ public class ApplyRuleJob extends Job {
for (Plan plan : groupExpressionMatching) {
List<Plan> newPlans = rule.transform(plan,
context.getPlannerContext());
for (Plan newPlan : newPlans) {
- GroupExpression newGroupExpression =
context.getPlannerContext().getMemo()
+ Pair<Boolean, GroupExpression> pair =
context.getPlannerContext().getMemo()
.copyIn(newPlan, groupExpression.getOwnerGroup(),
rule.isRewrite());
+ if (!pair.first) {
+ continue;
+ }
+ GroupExpression newGroupExpression = pair.second;
+
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/rewrite/RewriteBottomUpJob.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/RewriteBottomUpJob.java
index 5a8e8de593..3d88f51750 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/RewriteBottomUpJob.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/RewriteBottomUpJob.java
@@ -17,6 +17,7 @@
package org.apache.doris.nereids.jobs.rewrite;
+import org.apache.doris.common.Pair;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.jobs.Job;
import org.apache.doris.nereids.jobs.JobContext;
@@ -81,12 +82,13 @@ public class RewriteBottomUpJob extends Job {
Preconditions.checkArgument(afters.size() == 1);
Plan after = afters.get(0);
if (after != before) {
- GroupExpression groupExpr = context.getPlannerContext()
+ Pair<Boolean, GroupExpression> pair =
context.getPlannerContext()
.getMemo()
.copyIn(after, group, rule.isRewrite());
- groupExpr.setApplied(rule);
- pushTask(new RewriteBottomUpJob(group, rules, context,
false));
- return;
+ if (pair.first) {
+ pushTask(new RewriteBottomUpJob(group, rules, context,
false));
+ return;
+ }
}
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/RewriteTopDownJob.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/RewriteTopDownJob.java
index 6d65ceae89..0258412e68 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/RewriteTopDownJob.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/RewriteTopDownJob.java
@@ -17,6 +17,7 @@
package org.apache.doris.nereids.jobs.rewrite;
+import org.apache.doris.common.Pair;
import org.apache.doris.nereids.jobs.Job;
import org.apache.doris.nereids.jobs.JobContext;
import org.apache.doris.nereids.jobs.JobType;
@@ -65,18 +66,25 @@ public class RewriteTopDownJob extends Job {
List<Rule> validRules = getValidRules(logicalExpression, rules);
for (Rule rule : validRules) {
+ Preconditions.checkArgument(rule.isRewrite(),
+ "in top down job, rules must be rewritable");
GroupExpressionMatching groupExpressionMatching
= new GroupExpressionMatching(rule.getPattern(),
logicalExpression);
+ //In topdown job, there must be only one matching plan.
+ //This `for` loop runs at most once.
for (Plan before : groupExpressionMatching) {
List<Plan> afters = rule.transform(before,
context.getPlannerContext());
Preconditions.checkArgument(afters.size() == 1);
Plan after = afters.get(0);
if (after != before) {
- GroupExpression expression = context.getPlannerContext()
+ Pair<Boolean, GroupExpression> pair =
context.getPlannerContext()
.getMemo().copyIn(after, group, rule.isRewrite());
- expression.setApplied(rule);
- pushTask(new RewriteTopDownJob(group, rules, context));
- return;
+ if (pair.first) {
+ //new group-expr replaced the origin group-expr in
`group`,
+ //run this rule against this `group` again.
+ pushTask(new RewriteTopDownJob(group, rules, context));
+ return;
+ }
}
}
}
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 e78737a618..aa96af697d 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
@@ -18,6 +18,7 @@
package org.apache.doris.nereids.memo;
import org.apache.doris.common.IdGenerator;
+import org.apache.doris.common.Pair;
import org.apache.doris.nereids.PlannerContext;
import org.apache.doris.nereids.properties.LogicalProperties;
import org.apache.doris.nereids.trees.expressions.Expression;
@@ -47,7 +48,7 @@ public class Memo {
private Group root;
public Memo(Plan plan) {
- root = copyIn(plan, null, false).getOwnerGroup();
+ root = copyIn(plan, null, false).second.getOwnerGroup();
}
public Group getRoot() {
@@ -69,12 +70,13 @@ public class Memo {
* @param node {@link Plan} or {@link Expression} to be added
* @param target target group to add node. null to generate new Group
* @param rewrite whether to rewrite the node to the target group
- * @return Reference of node in Memo
+ * @return a pair, in which the first element is true if a newly generated
groupExpression added into memo,
+ * and the second element is a reference of node in Memo
*/
- public GroupExpression copyIn(Plan node, @Nullable Group target, boolean
rewrite) {
+ public Pair<Boolean, GroupExpression> copyIn(Plan node, @Nullable Group
target, boolean rewrite) {
Optional<GroupExpression> groupExpr = node.getGroupExpression();
if (!rewrite && groupExpr.isPresent() &&
groupExpressions.containsKey(groupExpr.get())) {
- return groupExpr.get();
+ return new Pair(false, groupExpr.get());
}
List<Group> childrenGroups = Lists.newArrayList();
for (int i = 0; i < node.children().size(); i++) {
@@ -84,13 +86,14 @@ public class Memo {
} else if (child.getGroupExpression().isPresent()) {
childrenGroups.add(child.getGroupExpression().get().getOwnerGroup());
} else {
- childrenGroups.add(copyIn(child, null,
rewrite).getOwnerGroup());
+ childrenGroups.add(copyIn(child, null,
rewrite).second.getOwnerGroup());
}
}
node = replaceChildrenToGroupPlan(node, childrenGroups);
GroupExpression newGroupExpression = new GroupExpression(node);
newGroupExpression.setChildren(childrenGroups);
- return insertOrRewriteGroupExpression(newGroupExpression, target,
rewrite, node.getLogicalProperties());
+ return insertOrRewriteGroupExpression(newGroupExpression, target,
rewrite,
+ node.getLogicalProperties());
// TODO: need to derive logical property if generate new group.
currently we not copy logical plan into
}
@@ -127,9 +130,10 @@ public class Memo {
* @param groupExpression groupExpression to insert
* @param target target group to insert or rewrite groupExpression
* @param rewrite whether to rewrite the groupExpression to target group
- * @return existing groupExpression in memo or newly generated
groupExpression
+ * @return a pair, in which the first element is true if a newly generated
groupExpression added into memo,
+ * and the second element is a reference of node in Memo
*/
- private GroupExpression insertOrRewriteGroupExpression(GroupExpression
groupExpression, Group target,
+ private Pair<Boolean, GroupExpression>
insertOrRewriteGroupExpression(GroupExpression groupExpression, Group target,
boolean rewrite, LogicalProperties logicalProperties) {
GroupExpression existedGroupExpression =
groupExpressions.get(groupExpression);
if (existedGroupExpression != null) {
@@ -140,7 +144,7 @@ public class Memo {
if (rewrite) {
mergedGroup.setLogicalProperties(logicalProperties);
}
- return existedGroupExpression;
+ return new Pair(false, existedGroupExpression);
}
if (target != null) {
if (rewrite) {
@@ -155,7 +159,7 @@ public class Memo {
groups.add(group);
}
groupExpressions.put(groupExpression, groupExpression);
- return groupExpression;
+ return new Pair(true, groupExpression);
}
/**
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
