You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by mo...@apache.org on 2023/06/02 11:55:41 UTC

[doris] branch master updated: [fix](Nereids) should not inherit child's limit and offset when generate exchange node (#20373)

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

morrysnow 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 299c3dc396 [fix](Nereids) should not inherit child's limit and offset when generate exchange node (#20373)
299c3dc396 is described below

commit 299c3dc396853873667445578f237c42c112116d
Author: morrySnow <10...@users.noreply.github.com>
AuthorDate: Fri Jun 2 19:55:33 2023 +0800

    [fix](Nereids) should not inherit child's limit and offset when generate exchange node (#20373)
    
    in legacy planner, when we new exchange, it inherit its child's limit and offset.
    but in Nereids, we should not do this. because if we need set limit or offset, we will set it manually.
    In this PR, we use a new ctor of ExchangeNode to ensure not set limit or offset unexpected.
---
 .../glue/translator/PhysicalPlanTranslator.java    | 24 ++++++----------------
 .../org/apache/doris/planner/ExchangeNode.java     | 14 +++++++++++++
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java
index c9f9543bf2..5d2b3c8f36 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java
@@ -1788,7 +1788,7 @@ public class PhysicalPlanTranslator extends DefaultPlanVisitor<PlanFragment, Pla
             }
         }
 
-        ExchangeNode exchange = new ExchangeNode(context.nextPlanNodeId(), childFragment.getPlanRoot(), false);
+        ExchangeNode exchange = new ExchangeNode(context.nextPlanNodeId(), childFragment.getPlanRoot());
         exchange.setNumInstances(childFragment.getPlanRoot().getNumInstances());
         childFragment.setPlanRoot(exchange);
         updateLegacyPlanIdToPhysicalPlan(childFragment.getPlanRoot(), distribute);
@@ -1951,8 +1951,7 @@ public class PhysicalPlanTranslator extends DefaultPlanVisitor<PlanFragment, Pla
         PhysicalCTEProducer cteProducer = context.getCteProduceMap().get(cteId);
         Preconditions.checkState(cteProducer != null, "invalid cteProducer");
 
-        ExchangeNode exchangeNode = new ExchangeNode(context.nextPlanNodeId(),
-                multCastFragment.getPlanRoot(), false);
+        ExchangeNode exchangeNode = new ExchangeNode(context.nextPlanNodeId(), multCastFragment.getPlanRoot());
 
         DataStreamSink streamSink = new DataStreamSink(exchangeNode.getId());
         streamSink.setPartition(DataPartition.RANDOM);
@@ -2171,8 +2170,7 @@ public class PhysicalPlanTranslator extends DefaultPlanVisitor<PlanFragment, Pla
 
     private PlanFragment createParentFragment(PlanFragment childFragment, DataPartition parentPartition,
             PlanTranslatorContext context) {
-        ExchangeNode exchangeNode = new ExchangeNode(context.nextPlanNodeId(),
-                childFragment.getPlanRoot(), false);
+        ExchangeNode exchangeNode = new ExchangeNode(context.nextPlanNodeId(), childFragment.getPlanRoot());
         exchangeNode.setNumInstances(childFragment.getPlanRoot().getNumInstances());
         PlanFragment parentFragment = new PlanFragment(context.nextFragmentId(), exchangeNode, parentPartition);
         childFragment.setDestination(exchangeNode);
@@ -2186,7 +2184,7 @@ public class PhysicalPlanTranslator extends DefaultPlanVisitor<PlanFragment, Pla
             PlanTranslatorContext context) {
         PlanNode exchange = parent.getChild(childIdx);
         if (!(exchange instanceof ExchangeNode)) {
-            exchange = new ExchangeNode(context.nextPlanNodeId(), childFragment.getPlanRoot(), false);
+            exchange = new ExchangeNode(context.nextPlanNodeId(), childFragment.getPlanRoot());
             exchange.setNumInstances(childFragment.getPlanRoot().getNumInstances());
         }
         childFragment.setPlanRoot(exchange.getChild(0));
@@ -2198,8 +2196,7 @@ public class PhysicalPlanTranslator extends DefaultPlanVisitor<PlanFragment, Pla
     private void connectChildFragmentNotCheckExchangeNode(PlanNode parent, int childIdx,
             PlanFragment parentFragment, PlanFragment childFragment,
             PlanTranslatorContext context) {
-        PlanNode exchange = new ExchangeNode(
-                context.nextPlanNodeId(), childFragment.getPlanRoot(), false);
+        PlanNode exchange = new ExchangeNode(context.nextPlanNodeId(), childFragment.getPlanRoot());
         exchange.setNumInstances(childFragment.getPlanRoot().getNumInstances());
         childFragment.setPlanRoot(exchange.getChild(0));
         exchange.setFragment(parentFragment);
@@ -2218,8 +2215,7 @@ public class PhysicalPlanTranslator extends DefaultPlanVisitor<PlanFragment, Pla
         }
 
         // exchange node clones the behavior of its input, aside from the conjuncts
-        ExchangeNode mergePlan = new ExchangeNode(context.nextPlanNodeId(),
-                inputFragment.getPlanRoot(), false);
+        ExchangeNode mergePlan = new ExchangeNode(context.nextPlanNodeId(), inputFragment.getPlanRoot());
         DataPartition dataPartition = DataPartition.UNPARTITIONED;
         mergePlan.setNumInstances(inputFragment.getPlanRoot().getNumInstances());
         PlanFragment fragment = new PlanFragment(context.nextFragmentId(), mergePlan, dataPartition);
@@ -2371,14 +2367,6 @@ public class PhysicalPlanTranslator extends DefaultPlanVisitor<PlanFragment, Pla
         return fragment.isPartitioned() && fragment.getPlanRoot().getNumInstances() > 1;
     }
 
-    private boolean projectOnAgg(PhysicalProject project) {
-        PhysicalPlan child = (PhysicalPlan) project.child(0);
-        while (child instanceof PhysicalFilter || child instanceof PhysicalDistribute) {
-            child = (PhysicalPlan) child.child(0);
-        }
-        return child instanceof PhysicalHashAggregate;
-    }
-
     private boolean hasExprCalc(PhysicalProject<? extends Plan> project) {
         for (NamedExpression p : project.getProjects()) {
             if (p.children().size() > 1) {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/ExchangeNode.java b/fe/fe-core/src/main/java/org/apache/doris/planner/ExchangeNode.java
index ab82dad209..52d658498a 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/planner/ExchangeNode.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/planner/ExchangeNode.java
@@ -40,6 +40,8 @@ import com.google.common.collect.Lists;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
+import java.util.Collections;
+
 /**
  * Receiver side of a 1:n data stream. Logically, an ExchangeNode consumes the data
  * produced by its children. For each of the sending child nodes the actual data
@@ -62,6 +64,18 @@ public class ExchangeNode extends PlanNode {
     // exchange node. Null if this exchange does not merge sorted streams
     private SortInfo mergeInfo;
 
+    /**
+     * use for Nereids only.
+     */
+    public ExchangeNode(PlanNodeId id, PlanNode inputNode) {
+        super(id, inputNode, EXCHANGE_NODE, StatisticalType.EXCHANGE_NODE);
+        offset = 0;
+        limit = -1;
+        this.conjuncts = Collections.emptyList();
+        children.add(inputNode);
+        computeTupleIds();
+    }
+
     /**
      * Create ExchangeNode that consumes output of inputNode.
      * An ExchangeNode doesn't have an input node as a child, which is why we


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