From 8206252e0a554971b38067d97a7ed0dbc24c77a3 Mon Sep 17 00:00:00 2001
From: jakevin <jakevingoo@gmail.com>
Date: Fri, 27 Oct 2023 10:56:42 +0800
Subject: [PATCH] [Performance](Nereids): refactor GroupMatching (#25960)

Refactor GroupMatching to improve performance.
---
 .../pattern/GroupExpressionMatching.java      |  4 +-
 .../doris/nereids/pattern/GroupMatching.java  | 86 ++++---------------
 2 files changed, 19 insertions(+), 71 deletions(-)

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 e685f8484f..f73ddcc886 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
@@ -26,7 +26,6 @@ import org.apache.doris.nereids.trees.plans.Plan;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 
-import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 import java.util.NoSuchElementException;
@@ -150,8 +149,7 @@ public class GroupExpressionMatching implements Iterable<Plan> {
                 }
             }
 
-            List<Plan> matchingChildren = new ArrayList<>();
-            new GroupMatching(childPattern, childGroup).forEach(matchingChildren::add);
+            List<Plan> matchingChildren = GroupMatching.getAllMatchingPlans(childPattern, childGroup);
             return matchingChildren;
         }
 
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/pattern/GroupMatching.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/pattern/GroupMatching.java
index a76a07d7fd..b3e59590d3 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/pattern/GroupMatching.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/pattern/GroupMatching.java
@@ -22,85 +22,35 @@ import org.apache.doris.nereids.memo.GroupExpression;
 import org.apache.doris.nereids.trees.plans.GroupPlan;
 import org.apache.doris.nereids.trees.plans.Plan;
 
-import com.google.common.collect.Iterators;
-
 import java.util.ArrayList;
-import java.util.Iterator;
 import java.util.List;
-import java.util.NoSuchElementException;
-import java.util.Objects;
 
 /**
  * Get all pattern matching subtree in query plan from a group.
  */
-public class GroupMatching implements Iterable<Plan> {
-    private final Pattern pattern;
-    private final Group group;
-
-    public GroupMatching(Pattern pattern, Group group) {
-        this.pattern = Objects.requireNonNull(pattern);
-        this.group = Objects.requireNonNull(group);
-    }
-
-    public final Iterator<Plan> iterator() {
-        return new GroupIterator(pattern, group);
-    }
-
+public class GroupMatching {
     /**
-     * Iterator to get all subtrees from a group.
+     * Get all pattern matching subtree in query plan from a group.
      */
-    public static class GroupIterator implements Iterator<Plan> {
-        private final List<Iterator<Plan>> iterator;
-        private int iteratorIndex = 0;
-
-        /**
-         * Constructor.
-         *
-         * @param pattern pattern to match
-         * @param group group to be matched
-         */
-        public GroupIterator(Pattern<? extends Plan> pattern, Group group) {
-            this.iterator = new ArrayList<>();
-
-            if (pattern.isGroup() || pattern.isMultiGroup()) {
-                GroupPlan groupPlan = new GroupPlan(group);
-                if (((Pattern<Plan>) pattern).matchPredicates(groupPlan)) {
-                    this.iterator.add(Iterators.singletonIterator(groupPlan));
+    public static List<Plan> getAllMatchingPlans(Pattern pattern, Group group) {
+        List<Plan> matchingPlans = new ArrayList<>();
+        if (pattern.isGroup() || pattern.isMultiGroup()) {
+            GroupPlan groupPlan = new GroupPlan(group);
+            if (((Pattern<Plan>) pattern).matchPredicates(groupPlan)) {
+                matchingPlans.add(groupPlan);
+            }
+        } else {
+            for (GroupExpression groupExpression : group.getLogicalExpressions()) {
+                for (Plan plan : new GroupExpressionMatching(pattern, groupExpression)) {
+                    matchingPlans.add(plan);
                 }
-            } else {
-                for (GroupExpression groupExpression : group.getLogicalExpressions()) {
-                    GroupExpressionMatching.GroupExpressionIterator groupExpressionIterator =
-                            new GroupExpressionMatching(pattern, groupExpression).iterator();
-                    if (groupExpressionIterator.hasNext()) {
-                        this.iterator.add(groupExpressionIterator);
-                    }
-                }
-                for (GroupExpression groupExpression : group.getPhysicalExpressions()) {
-                    GroupExpressionMatching.GroupExpressionIterator groupExpressionIterator =
-                            new GroupExpressionMatching(pattern, groupExpression).iterator();
-                    if (groupExpressionIterator.hasNext()) {
-                        this.iterator.add(groupExpressionIterator);
-                    }
+            }
+            for (GroupExpression groupExpression : group.getPhysicalExpressions()) {
+                for (Plan plan : new GroupExpressionMatching(pattern, groupExpression)) {
+                    matchingPlans.add(plan);
                 }
             }
         }
-
-        @Override
-        public final boolean hasNext() {
-            return iteratorIndex < iterator.size();
-        }
-
-        @Override
-        public final Plan next() {
-            if (!hasNext()) {
-                throw new NoSuchElementException();
-            }
-
-            Plan result = iterator.get(iteratorIndex).next();
-            if (!iterator.get(iteratorIndex).hasNext()) {
-                iteratorIndex++;
-            }
-            return result;
-        }
+        return matchingPlans;
     }
 }