You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by ja...@apache.org on 2023/01/19 03:13:30 UTC

[doris] branch master updated: [enhance](Nereids): remove Group constructor for UT. (#16005)

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 e846e8c0fd [enhance](Nereids): remove Group constructor for UT. (#16005)
e846e8c0fd is described below

commit e846e8c0fd0324ae6651f4689a2ffed4031092ec
Author: jakevin <ja...@gmail.com>
AuthorDate: Thu Jan 19 11:13:23 2023 +0800

    [enhance](Nereids): remove Group constructor for UT. (#16005)
---
 .../org/apache/doris/nereids/NereidsPlanner.java   |  7 ++---
 .../jobs/joinorder/hypergraph/GraphSimplifier.java |  6 ++--
 .../hypergraph/receiver/PlanReceiver.java          | 10 +++---
 .../java/org/apache/doris/nereids/memo/Group.java  |  7 -----
 .../java/org/apache/doris/nereids/memo/Memo.java   |  2 --
 .../doris/nereids/stats/StatsCalculatorTest.java   | 36 +++++++++++++---------
 .../org/apache/doris/nereids/util/PlanChecker.java | 10 ++----
 7 files changed, 36 insertions(+), 42 deletions(-)

diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java
index a29803ab3b..c4fb5b9b7f 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java
@@ -225,10 +225,9 @@ public class NereidsPlanner extends Planner {
             // To keep the root group is not changed, we add a project operator above join
             List<Slot> outputs = root.getLogicalExpression().getPlan().getOutput();
             GroupExpression newExpr = new GroupExpression(
-                    new LogicalProject(outputs, root.getLogicalExpression().getPlan()),
-                    Lists.newArrayList(root));
-            root = new Group();
-            root.addGroupExpression(newExpr);
+                    new LogicalProject(outputs, root.getLogicalExpression().getPlan()), Lists.newArrayList(root));
+            // FIXME: use wrong constructor.
+            root = new Group(null, newExpr, null);
             changeRoot = true;
         }
         cascadesContext.pushJob(new JoinOrderJob(root, cascadesContext.getCurrentJobContext()));
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraph/GraphSimplifier.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraph/GraphSimplifier.java
index c73a2d9b08..80c89185f0 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraph/GraphSimplifier.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraph/GraphSimplifier.java
@@ -425,14 +425,14 @@ public class GraphSimplifier {
         //      Plan.cost = Plan.rowCount + Plan.children1.cost + Plan.children2.cost
         // The reason is that this cost model has ASI (adjacent sequence interchange) property.
         // TODO: consider network, data distribution cost
-        LogicalJoin newJoin = new LogicalJoin(join.getJoinType(), plan1, plan2);
+        LogicalJoin newJoin = new LogicalJoin<>(join.getJoinType(), plan1, plan2);
         List<Group> children = new ArrayList<>();
         children.add(getGroup(plan1));
         children.add(getGroup(plan2));
         double cost = getSimpleCost(plan1) + getSimpleCost(plan2);
         GroupExpression groupExpression = new GroupExpression(newJoin, children);
-        Group group = new Group();
-        group.addGroupExpression(groupExpression);
+        // FIXME: use wrong constructor
+        Group group = new Group(null, groupExpression, null);
         StatsCalculator.estimate(groupExpression);
         cost += group.getStatistics().getRowCount();
 
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraph/receiver/PlanReceiver.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraph/receiver/PlanReceiver.java
index 26f6c846ca..2fb98b08de 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraph/receiver/PlanReceiver.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraph/receiver/PlanReceiver.java
@@ -140,8 +140,8 @@ public class PlanReceiver implements AbstractReceiver {
             groupExpression.updateLowestCostTable(PhysicalProperties.ANY,
                     Lists.newArrayList(PhysicalProperties.ANY, PhysicalProperties.ANY),
                     group.getLogicalExpression().getCostByProperties(PhysicalProperties.ANY));
-            Group projectGroup = new Group();
-            projectGroup.addGroupExpression(groupExpression);
+            // FIXME: use wrong constructor
+            Group projectGroup = new Group(null, groupExpression, null);
             StatsCalculator.estimate(groupExpression);
             return projectGroup;
         }
@@ -159,13 +159,13 @@ public class PlanReceiver implements AbstractReceiver {
         for (Edge edge : edges) {
             conditions.addAll(edge.getJoin().getExpressions());
         }
-        LogicalJoin newJoin = new LogicalJoin(edges.get(0).getJoin().getJoinType(), conditions,
+        LogicalJoin newJoin = new LogicalJoin<>(edges.get(0).getJoin().getJoinType(), conditions,
                 leftGroup.getLogicalExpression().getPlan(),
                 rightGroup.getLogicalExpression().getPlan());
 
         GroupExpression groupExpression = new GroupExpression(newJoin, Lists.newArrayList(leftGroup, rightGroup));
-        Group group = new Group();
-        group.addGroupExpression(groupExpression);
+        // FIXME: use wrong constructor
+        Group group = new Group(null, groupExpression, null);
         StatsCalculator.estimate(groupExpression);
         cost += group.getStatistics().getRowCount();
 
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 7077f4d9d2..afb77f4188 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
@@ -74,13 +74,6 @@ public class Group {
         this.logicalProperties = logicalProperties;
     }
 
-    /**
-     * For unit test only.
-     */
-    public Group() {
-        groupId = null;
-    }
-
     public GroupId getGroupId() {
         return groupId;
     }
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 b51e69318b..83ba68f88a 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
@@ -84,8 +84,6 @@ public class Memo {
     /**
      * This function used to update the root group when DPHyp change the root Group
      * Note it only used in DPHyp
-     *
-     * @param root The new root Group
      */
     public void setRoot(Group root) {
         this.root = root;
diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/stats/StatsCalculatorTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/stats/StatsCalculatorTest.java
index fd9dfe8fcd..e931957be2 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/nereids/stats/StatsCalculatorTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/stats/StatsCalculatorTest.java
@@ -26,6 +26,7 @@ import org.apache.doris.nereids.trees.expressions.EqualTo;
 import org.apache.doris.nereids.trees.expressions.Or;
 import org.apache.doris.nereids.trees.expressions.SlotReference;
 import org.apache.doris.nereids.trees.expressions.literal.IntegerLiteral;
+import org.apache.doris.nereids.trees.plans.FakePlan;
 import org.apache.doris.nereids.trees.plans.GroupPlan;
 import org.apache.doris.nereids.trees.plans.logical.LogicalFilter;
 import org.apache.doris.nereids.trees.plans.logical.LogicalLimit;
@@ -55,6 +56,13 @@ import java.util.Optional;
 
 public class StatsCalculatorTest {
 
+    private Group newGroup() {
+        GroupExpression groupExpression = new GroupExpression(new FakePlan());
+        Group group = new Group(null, groupExpression, null);
+        group.getPhysicalExpressions().remove(0);
+        return group;
+    }
+
     // TODO: temporary disable this test, until we could get column stats
     // @Test
     // public void testAgg() {
@@ -77,7 +85,7 @@ public class StatsCalculatorTest {
     //     AggregateFunction sum = new Sum(slot2);
     //     StatsDeriveResult childStats = new StatsDeriveResult(20, slotColumnStatsMap);
     //     Alias alias = new Alias(sum, "a");
-    //     Group childGroup = new Group();
+    //     Group childGroup = newGroup();
     //     childGroup.setLogicalProperties(new LogicalProperties(new Supplier<List<Slot>>() {
     //         @Override
     //         public List<Slot> get() {
@@ -88,7 +96,7 @@ public class StatsCalculatorTest {
     //     childGroup.setStatistics(childStats);
     //     LogicalAggregate logicalAggregate = new LogicalAggregate(groupByExprList, Arrays.asList(alias), groupPlan);
     //     GroupExpression groupExpression = new GroupExpression(logicalAggregate, Arrays.asList(childGroup));
-    //     Group ownerGroup = new Group();
+    //     Group ownerGroup = newGroup();
     //     groupExpression.setOwnerGroup(ownerGroup);
     //     StatsCalculator.estimate(groupExpression);
     //     Assertions.assertEquals(groupExpression.getOwnerGroup().getStatistics().getRowCount(), 10);
@@ -124,21 +132,21 @@ public class StatsCalculatorTest {
         ImmutableSet and = ImmutableSet.of(eq1, eq2);
         ImmutableSet or = ImmutableSet.of(new Or(eq1, eq2));
 
-        Group childGroup = new Group();
+        Group childGroup = newGroup();
         childGroup.setLogicalProperties(new LogicalProperties(Collections::emptyList));
         GroupPlan groupPlan = new GroupPlan(childGroup);
         childGroup.setStatistics(childStats);
 
         LogicalFilter<GroupPlan> logicalFilter = new LogicalFilter<>(and, groupPlan);
         GroupExpression groupExpression = new GroupExpression(logicalFilter, ImmutableList.of(childGroup));
-        Group ownerGroup = new Group();
+        Group ownerGroup = newGroup();
         groupExpression.setOwnerGroup(ownerGroup);
         StatsCalculator.estimate(groupExpression);
         Assertions.assertEquals((long) (10000 * 0.1 * 0.05), ownerGroup.getStatistics().getRowCount(), 0.001);
 
         LogicalFilter<GroupPlan> logicalFilterOr = new LogicalFilter<>(or, groupPlan);
         GroupExpression groupExpressionOr = new GroupExpression(logicalFilterOr, ImmutableList.of(childGroup));
-        Group ownerGroupOr = new Group();
+        Group ownerGroupOr = newGroup();
         groupExpressionOr.setOwnerGroup(ownerGroupOr);
         StatsCalculator.estimate(groupExpressionOr);
         Assertions.assertEquals((long) (10000 * (0.1 + 0.05 - 0.1 * 0.05)),
@@ -177,21 +185,21 @@ public class StatsCalculatorTest {
         ImmutableSet and = ImmutableSet.of(eq1, eq2);
         ImmutableSet or = ImmutableSet.of(new Or(eq1, eq2));
 
-        Group childGroup = new Group();
+        Group childGroup = newGroup();
         childGroup.setLogicalProperties(new LogicalProperties(Collections::emptyList));
         GroupPlan groupPlan = new GroupPlan(childGroup);
         childGroup.setStatistics(childStats);
 
         LogicalFilter<GroupPlan> logicalFilter = new LogicalFilter<>(and, groupPlan);
         GroupExpression groupExpression = new GroupExpression(logicalFilter, ImmutableList.of(childGroup));
-        Group ownerGroup = new Group();
+        Group ownerGroup = newGroup();
         groupExpression.setOwnerGroup(ownerGroup);
         StatsCalculator.estimate(groupExpression);
         Assertions.assertEquals(0, ownerGroup.getStatistics().getRowCount(), 0.001);
 
         LogicalFilter<GroupPlan> logicalFilterOr = new LogicalFilter<>(or, groupPlan);
         GroupExpression groupExpressionOr = new GroupExpression(logicalFilterOr, ImmutableList.of(childGroup));
-        Group ownerGroupOr = new Group();
+        Group ownerGroupOr = newGroup();
         groupExpressionOr.setOwnerGroup(ownerGroupOr);
         StatsCalculator.estimate(groupExpressionOr);
         Assertions.assertEquals(0, ownerGroupOr.getStatistics().getRowCount(), 0.001);
@@ -243,9 +251,9 @@ public class StatsCalculatorTest {
         OlapTable table1 = PlanConstructor.newOlapTable(tableId1, "t1", 0);
         LogicalOlapScan logicalOlapScan1 = new LogicalOlapScan(RelationUtil.newRelationId(), table1, Collections.emptyList())
                 .withLogicalProperties(Optional.of(new LogicalProperties(() -> ImmutableList.of(slot1))));
-        Group childGroup = new Group();
+        Group childGroup = newGroup();
         GroupExpression groupExpression = new GroupExpression(logicalOlapScan1, ImmutableList.of(childGroup));
-        Group ownerGroup = new Group();
+        Group ownerGroup = newGroup();
         groupExpression.setOwnerGroup(ownerGroup);
         StatsCalculator.estimate(groupExpression);
         StatsDeriveResult stats = ownerGroup.getStatistics();
@@ -266,14 +274,14 @@ public class StatsCalculatorTest {
         slotColumnStatsMap.put(slot1.getExprId(), columnStat1.build());
         StatsDeriveResult childStats = new StatsDeriveResult(10, slotColumnStatsMap);
 
-        Group childGroup = new Group();
+        Group childGroup = newGroup();
         childGroup.setLogicalProperties(new LogicalProperties(Collections::emptyList));
         GroupPlan groupPlan = new GroupPlan(childGroup);
         childGroup.setStatistics(childStats);
 
         LogicalLimit<GroupPlan> logicalLimit = new LogicalLimit<>(1, 2, groupPlan);
         GroupExpression groupExpression = new GroupExpression(logicalLimit, ImmutableList.of(childGroup));
-        Group ownerGroup = new Group();
+        Group ownerGroup = newGroup();
         ownerGroup.addGroupExpression(groupExpression);
         StatsCalculator.estimate(groupExpression);
         StatsDeriveResult limitStats = ownerGroup.getStatistics();
@@ -296,14 +304,14 @@ public class StatsCalculatorTest {
         slotColumnStatsMap.put(slot1.getExprId(), columnStat1.build());
         StatsDeriveResult childStats = new StatsDeriveResult(10, slotColumnStatsMap);
 
-        Group childGroup = new Group();
+        Group childGroup = newGroup();
         childGroup.setLogicalProperties(new LogicalProperties(Collections::emptyList));
         GroupPlan groupPlan = new GroupPlan(childGroup);
         childGroup.setStatistics(childStats);
 
         LogicalTopN<GroupPlan> logicalTopN = new LogicalTopN<>(Collections.emptyList(), 1, 2, groupPlan);
         GroupExpression groupExpression = new GroupExpression(logicalTopN, ImmutableList.of(childGroup));
-        Group ownerGroup = new Group();
+        Group ownerGroup = newGroup();
         ownerGroup.addGroupExpression(groupExpression);
         StatsCalculator.estimate(groupExpression);
         StatsDeriveResult topNStats = ownerGroup.getStatistics();
diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/util/PlanChecker.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/util/PlanChecker.java
index df62789bfc..225cf736be 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/nereids/util/PlanChecker.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/util/PlanChecker.java
@@ -325,11 +325,12 @@ public class PlanChecker {
         boolean changeRoot = false;
         if (root.isJoinGroup()) {
             List<Slot> outputs = root.getLogicalExpression().getPlan().getOutput();
+            // FIXME: can't match type, convert List<Slot> to List<NamedExpression>
             GroupExpression newExpr = new GroupExpression(
                     new LogicalProject(outputs, root.getLogicalExpression().getPlan()),
                     Lists.newArrayList(root));
-            root = new Group();
-            root.addGroupExpression(newExpr);
+            // FIXME: use wrong constructor.
+            root = new Group(null, newExpr, null);
             changeRoot = true;
         }
         cascadesContext.pushJob(new JoinOrderJob(root, cascadesContext.getCurrentJobContext()));
@@ -373,11 +374,6 @@ public class PlanChecker {
         return this;
     }
 
-    public PlanChecker checkCascadesContext(Consumer<CascadesContext> contextChecker) {
-        contextChecker.accept(cascadesContext);
-        return this;
-    }
-
     public PlanChecker checkGroupNum(int expectGroupNum) {
         Assertions.assertEquals(expectGroupNum, cascadesContext.getMemo().getGroups().size());
         return this;


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