This is an automated email from the ASF dual-hosted git repository.

jakevin 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 53814e466b [Enhancement](Nereids)optimize merge group in memo #13900
53814e466b is described below

commit 53814e466b67e40a7f8656484c6fe38c9040d369
Author: mch_ucchi <41606806+sohardforan...@users.noreply.github.com>
AuthorDate: Wed Nov 2 20:42:55 2022 +0800

    [Enhancement](Nereids)optimize merge group in memo #13900
---
 .../apache/doris/nereids/memo/GroupExpression.java | 23 +++++++++++++++++++---
 .../java/org/apache/doris/nereids/memo/Memo.java   | 23 +++++-----------------
 2 files changed, 25 insertions(+), 21 deletions(-)

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 92067f607d..1972e71961 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
@@ -28,6 +28,7 @@ import org.apache.doris.statistics.StatsDeriveResult;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableList.Builder;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 
@@ -106,7 +107,9 @@ public class GroupExpression {
     }
 
     public void setChildren(ImmutableList<Group> children) {
+        this.children.forEach(g -> g.removeParentExpression(this));
         this.children = children;
+        this.children.forEach(g -> g.addParentExpression(this));
     }
 
     /**
@@ -117,22 +120,36 @@ public class GroupExpression {
      */
     public void replaceChild(Group originChild, Group newChild) {
         originChild.removeParentExpression(this);
+        List<Group> groups = 
Lists.newArrayListWithCapacity(this.children.size());
         for (int i = 0; i < children.size(); i++) {
             if (children.get(i) == originChild) {
-                children.set(i, newChild);
-                newChild.addParentExpression(this);
+                groups.add(newChild);
+            } else {
+                groups.add(child(i));
             }
         }
+        children = ImmutableList.copyOf(groups);
+        newChild.addParentExpression(this);
     }
 
     public void setChild(int index, Group group) {
-        this.children.set(index, group);
+        this.children.get(index).removeParentExpression(this);
+        setChildByIndex(index, group);
     }
 
     public boolean hasApplied(Rule rule) {
         return ruleMasks.get(rule.getRuleType().ordinal());
     }
 
+    private void setChildByIndex(int index, Group group) {
+        ImmutableList.Builder<Group> builder = new Builder<>();
+        builder.addAll(children.subList(0, index));
+        builder.add(group);
+        builder.addAll(children.subList(index + 1, children.size()));
+        children = builder.build();
+        group.addParentExpression(this);
+    }
+
     public boolean notApplied(Rule rule) {
         return !hasApplied(rule);
     }
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 f5616a71c7..3583db9f4e 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
@@ -375,26 +375,13 @@ public class Memo {
         if (source.equals(destination)) {
             return source;
         }
-        List<GroupExpression> needReplaceChild = Lists.newArrayList();
-        for (GroupExpression groupExpression : groupExpressions.values()) {
-            if (groupExpression.children().contains(source)) {
-                if (groupExpression.getOwnerGroup().equals(destination)) {
-                    // cycle, we should not merge
-                    return null;
-                }
-                needReplaceChild.add(groupExpression);
-            }
+        if (source.getParentGroupExpressions().stream()
+                .anyMatch(e -> e.getOwnerGroup().equals(destination))) {
+            return null;
         }
-        for (GroupExpression groupExpression : needReplaceChild) {
+        for (GroupExpression groupExpression : 
source.getParentGroupExpressions()) {
             groupExpressions.remove(groupExpression);
-            List<Group> children = new ArrayList<>(groupExpression.children());
-            // TODO: use a better way to replace child, avoid traversing all 
groupExpression
-            for (int i = 0; i < children.size(); i++) {
-                if (children.get(i).equals(source)) {
-                    children.set(i, destination);
-                }
-            }
-            groupExpression.setChildren(ImmutableList.copyOf(children));
+            groupExpression.replaceChild(source, destination);
 
             GroupExpression that = groupExpressions.get(groupExpression);
             if (that != null && that.getOwnerGroup() != null


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to