From e307884e57fc93960a803bd9065dda4c57237bd5 Mon Sep 17 00:00:00 2001 From: jakevin Date: Thu, 2 Nov 2023 21:35:48 +0800 Subject: [PATCH] [enhancement](Nereids): GroupPlan don't generate ObjectId (#26315) --- .../doris/nereids/memo/GroupExpression.java | 4 +++- .../nereids/trees/plans/AbstractPlan.java | 21 ++++++++++++++----- .../doris/nereids/trees/plans/GroupPlan.java | 3 ++- .../nereids/trees/plans/commands/Command.java | 2 +- .../plans/logical/AbstractLogicalPlan.java | 9 +++++++- .../trees/plans/logical/LogicalLeaf.java | 10 +++++++-- 6 files changed, 38 insertions(+), 11 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 8eabceb425..eda7f5c9c3 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 @@ -97,11 +97,13 @@ public class GroupExpression { this.plan = Objects.requireNonNull(plan, "plan can not be null") .withGroupExpression(Optional.of(this)); this.children = Objects.requireNonNull(children, "children can not be null"); - this.children.forEach(childGroup -> childGroup.addParentExpression(this)); this.ruleMasks = new BitSet(RuleType.SENTINEL.ordinal()); this.statDerived = false; this.lowestCostTable = Maps.newHashMap(); this.requestPropertiesMap = Maps.newHashMap(); + for (Group child : children) { + child.addParentExpression(this); + } } public PhysicalProperties getOutputProperties(PhysicalProperties requestProperties) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/AbstractPlan.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/AbstractPlan.java index c223dd43b6..1e4fbdcf41 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/AbstractPlan.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/AbstractPlan.java @@ -31,6 +31,7 @@ import org.apache.doris.nereids.util.TreeStringUtils; import org.apache.doris.planner.PlanNodeId; import org.apache.doris.statistics.Statistics; +import com.google.common.base.Preconditions; import com.google.common.base.Supplier; import com.google.common.base.Suppliers; import org.json.JSONArray; @@ -47,7 +48,8 @@ import javax.annotation.Nullable; */ public abstract class AbstractPlan extends AbstractTreeNode implements Plan { public static final String FRAGMENT_ID = "fragment"; - protected final ObjectId id = StatementScopeIdGenerator.newObjectId(); + private static final ObjectId zeroId = new ObjectId(0); + protected final ObjectId id; protected final Statistics statistics; protected final PlanType type; @@ -61,10 +63,6 @@ public abstract class AbstractPlan extends AbstractTreeNode implements Pla // difficult to locate. private MutableState mutableState = EmptyMutableState.INSTANCE; - protected AbstractPlan(PlanType type, List children) { - this(type, Optional.empty(), Optional.empty(), null, children); - } - /** * all parameter constructor. */ @@ -78,6 +76,19 @@ public abstract class AbstractPlan extends AbstractTreeNode implements Pla this.logicalPropertiesSupplier = Suppliers.memoize(() -> optLogicalProperties.orElseGet( this::computeLogicalProperties)); this.statistics = statistics; + this.id = StatementScopeIdGenerator.newObjectId(); + } + + protected AbstractPlan(PlanType type, Optional groupExpression, + Supplier logicalPropertiesSupplier, @Nullable Statistics statistics, + List children, boolean useZeroId) { + super(children); + this.type = Objects.requireNonNull(type, "type can not be null"); + this.groupExpression = Objects.requireNonNull(groupExpression, "groupExpression can not be null"); + this.logicalPropertiesSupplier = logicalPropertiesSupplier; + this.statistics = statistics; + Preconditions.checkArgument(useZeroId); + this.id = zeroId; } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/GroupPlan.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/GroupPlan.java index bbd5f8afb2..0c3f519038 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/GroupPlan.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/GroupPlan.java @@ -26,6 +26,7 @@ import org.apache.doris.nereids.trees.plans.logical.LogicalLeaf; import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor; import org.apache.doris.statistics.Statistics; +import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableList; import java.util.List; @@ -41,7 +42,7 @@ public class GroupPlan extends LogicalLeaf { private final Group group; public GroupPlan(Group group) { - super(PlanType.GROUP_PLAN, Optional.empty(), Optional.of(group.getLogicalProperties())); + super(PlanType.GROUP_PLAN, Optional.empty(), Suppliers.ofInstance(group.getLogicalProperties()), true); this.group = group; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/Command.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/Command.java index af48fe3425..2adc9d9042 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/Command.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/Command.java @@ -39,7 +39,7 @@ import java.util.Optional; public abstract class Command extends AbstractPlan implements LogicalPlan { protected Command(PlanType type) { - super(type, ImmutableList.of()); + super(type, Optional.empty(), Optional.empty(), null, ImmutableList.of()); } public void run(ConnectContext ctx, StmtExecutor executor) throws Exception { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/AbstractLogicalPlan.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/AbstractLogicalPlan.java index e2a65770f5..0f59b38bc9 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/AbstractLogicalPlan.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/AbstractLogicalPlan.java @@ -25,6 +25,7 @@ import org.apache.doris.nereids.trees.plans.Plan; import org.apache.doris.nereids.trees.plans.PlanType; import org.apache.doris.qe.ConnectContext; +import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; import java.util.List; @@ -35,7 +36,7 @@ import java.util.Optional; */ public abstract class AbstractLogicalPlan extends AbstractPlan implements LogicalPlan, Explainable { protected AbstractLogicalPlan(PlanType type, List children) { - super(type, children); + this(type, Optional.empty(), Optional.empty(), children); } protected AbstractLogicalPlan(PlanType type, Optional groupExpression, @@ -48,6 +49,12 @@ public abstract class AbstractLogicalPlan extends AbstractPlan implements Logica super(type, groupExpression, logicalProperties, null, children); } + // Don't generate ObjectId for LogicalPlan + protected AbstractLogicalPlan(PlanType type, Optional groupExpression, + Supplier logicalPropertiesSupplier, List children, boolean useZeroId) { + super(type, groupExpression, logicalPropertiesSupplier, null, children, useZeroId); + } + @Override public Plan getExplainPlan(ConnectContext ctx) { return this; diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalLeaf.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalLeaf.java index a87f408bf3..68dabd0928 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalLeaf.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalLeaf.java @@ -23,6 +23,7 @@ import org.apache.doris.nereids.trees.expressions.Slot; import org.apache.doris.nereids.trees.plans.LeafPlan; import org.apache.doris.nereids.trees.plans.PlanType; +import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; import java.util.List; @@ -33,10 +34,15 @@ import java.util.Optional; */ public abstract class LogicalLeaf extends AbstractLogicalPlan implements LeafPlan, OutputSavePoint { - public LogicalLeaf(PlanType nodeType, Optional groupExpression, - Optional logicalProperties) { + protected LogicalLeaf(PlanType nodeType, Optional groupExpression, + Optional logicalProperties) { super(nodeType, groupExpression, logicalProperties, ImmutableList.of()); } + protected LogicalLeaf(PlanType nodeType, Optional groupExpression, + Supplier logicalPropertiesSupplier, boolean useZeroId) { + super(nodeType, groupExpression, logicalPropertiesSupplier, ImmutableList.of(), useZeroId); + } + public abstract List computeOutput(); }