You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/07/28 08:15:42 UTC

[GitHub] [doris] morrySnow commented on a diff in pull request #11222: [feature](nereids): polish property deriver enforcer job

morrySnow commented on code in PR #11222:
URL: https://github.com/apache/doris/pull/11222#discussion_r931764521


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/DistributionSpec.java:
##########
@@ -21,52 +21,38 @@
 import org.apache.doris.nereids.memo.GroupExpression;
 import org.apache.doris.nereids.trees.plans.GroupPlan;
 import org.apache.doris.nereids.trees.plans.physical.PhysicalDistribution;
-import org.apache.doris.planner.DataPartition;
 
 import com.google.common.collect.Lists;
 
 /**
  * Spec of data distribution.
+ * GPORCA has more type in CDistributionSpec.
  */
-public class DistributionSpec {
-
-    private DataPartition dataPartition;
-
-    // TODO: why exist?
-    public DistributionSpec() {
-    }
-
-    public DistributionSpec(DataPartition dataPartition) {
-        this.dataPartition = dataPartition;
-    }
-
+public abstract class DistributionSpec {
     /**
      * TODO: need read ORCA.
      * Whether other `DistributionSpec` is satisfied the current `DistributionSpec`.
      *
      * @param other another DistributionSpec.
      */
-    public boolean meet(DistributionSpec other) {
-        return false;
-    }
-
-    public DataPartition getDataPartition() {
-        return dataPartition;
-    }
-
-    public void setDataPartition(DataPartition dataPartition) {
-        this.dataPartition = dataPartition;
-    }
+    public abstract boolean meet(DistributionSpec other);
 
     public GroupExpression addEnforcer(Group child) {
         PhysicalDistribution distribution = new PhysicalDistribution(
-                new DistributionSpec(dataPartition), child.getLogicalProperties(), new GroupPlan(child));
+                this,
+                child.getLogicalProperties(),

Review Comment:
   i doubt u need to new a LogicalProperties or just do not set logical properties for this node. If we don't set LogicalProperties explicitly, the node will compute a applicable LogicalProperties for itself.



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/DistributionSpecHash.java:
##########
@@ -0,0 +1,102 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.properties;
+
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+
+import java.util.List;
+
+/**
+ * Describe hash distribution.
+ */
+public class DistributionSpecHash extends DistributionSpec {
+
+    private final List<SlotReference> shuffledColumns;
+    private final ShuffleType shuffleType;
+    private PropertyInfo propertyInfo;
+
+    public DistributionSpecHash(List<SlotReference> shuffledColumns, ShuffleType shuffleType) {
+        Preconditions.checkState(!shuffledColumns.isEmpty());
+        this.shuffledColumns = shuffledColumns;
+        this.shuffleType = shuffleType;
+    }
+
+    public DistributionSpecHash(List<SlotReference> shuffledColumns, ShuffleType shuffleType,
+            PropertyInfo propertyInfo) {
+        Preconditions.checkState(!shuffledColumns.isEmpty());
+        this.shuffledColumns = shuffledColumns;
+        this.shuffleType = shuffleType;
+        this.propertyInfo = propertyInfo;
+    }
+
+    public List<SlotReference> getShuffledColumns() {
+        return shuffledColumns;
+    }
+
+    public ShuffleType getShuffleType() {
+        return shuffleType;
+    }
+
+    public PropertyInfo getPropertyInfo() {
+        return propertyInfo;
+    }
+
+    @Override
+    public boolean meet(DistributionSpec other) {
+        if (other instanceof DistributionSpecAny) {
+            return true;
+        }
+
+        if (!(other instanceof DistributionSpecHash)) {
+            return false;
+        }
+
+        // DistributionSpecHash spec = (DistributionSpecHash) other;

Review Comment:
   add a TODO and add @Developing annotation on class



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/cost/CostCalculator.java:
##########
@@ -62,41 +64,73 @@ public CostEstimate visit(Plan plan, PlanContext context) {
         }
 
         @Override
-        public CostEstimate visitPhysicalAggregate(PhysicalAggregate<Plan> aggregate, PlanContext context) {
+        public CostEstimate visitPhysicalOlapScan(PhysicalOlapScan physicalOlapScan, PlanContext context) {
             StatsDeriveResult statistics = context.getStatisticsWithCheck();
             return CostEstimate.ofCpu(statistics.computeSize());
         }
 
         @Override
-        public CostEstimate visitPhysicalOlapScan(PhysicalOlapScan physicalOlapScan, PlanContext context) {
+        public CostEstimate visitPhysicalProject(PhysicalProject physicalProject, PlanContext context) {
             StatsDeriveResult statistics = context.getStatisticsWithCheck();
             return CostEstimate.ofCpu(statistics.computeSize());
         }
 
+        @Override
+        public CostEstimate visitPhysicalHeapSort(PhysicalHeapSort physicalHeapSort, PlanContext context) {
+            // TODO: consider two-phase sort and enforcer.
+            StatsDeriveResult statistics = context.getStatisticsWithCheck();
+            StatsDeriveResult childStatistics = context.getChildStatistics(0);
+
+            return new CostEstimate(
+                    childStatistics.computeSize(),
+                    statistics.computeSize(),
+                    childStatistics.computeSize());
+        }
+
+        @Override
+        public CostEstimate visitPhysicalDistribution(PhysicalDistribution physicalDistribution, PlanContext context) {
+            StatsDeriveResult statistics = context.getStatisticsWithCheck();
+            StatsDeriveResult childStatistics = context.getChildStatistics(0);
+
+            return new CostEstimate(
+                    childStatistics.computeSize(),
+                    statistics.computeSize(),
+                    childStatistics.computeSize());
+        }
+
+        @Override
+        public CostEstimate visitPhysicalAggregate(PhysicalAggregate<Plan> aggregate, PlanContext context) {
+            // TODO: stage.....
+
+            StatsDeriveResult statistics = context.getStatisticsWithCheck();
+            StatsDeriveResult inputStatistics = context.getChildStatistics(0);
+            return new CostEstimate(inputStatistics.computeSize(), statistics.computeSize(), 0);
+        }
+
         @Override
         public CostEstimate visitPhysicalHashJoin(PhysicalHashJoin<Plan, Plan> physicalHashJoin, PlanContext context) {
             Preconditions.checkState(context.getGroupExpression().arity() == 2);
             Preconditions.checkState(context.getChildrenStats().size() == 2);
 
             StatsDeriveResult leftStatistics = context.getChildStatistics(0);
             StatsDeriveResult rightStatistics = context.getChildStatistics(1);
-
-            // TODO: handle some case
-
             List<Id> leftIds = context.getChildOutputIds(0);
             List<Id> rightIds = context.getChildOutputIds(1);
 
+            // TODO: handle some case
             // handle cross join, onClause is empty .....
+            if (physicalHashJoin.getJoinType().isCrossJoin()) {
+                return new CostEstimate(
+                        leftStatistics.computeColumnSize(leftIds) + rightStatistics.computeColumnSize(rightIds),
+                        rightStatistics.computeColumnSize(rightIds),
+                        0);
+            }
 
             return new CostEstimate(
-                    leftStatistics.computeColumnSize(leftIds) + rightStatistics.computeColumnSize(rightIds),
-                    rightStatistics.computeColumnSize(rightIds), 0);
+                    (leftStatistics.computeColumnSize(leftIds) + rightStatistics.computeColumnSize(rightIds)) / 2,
+                    0,

Review Comment:
   why memory and network cost is zero?



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/DistributionSpecGather.java:
##########
@@ -20,9 +20,14 @@
 /**
  * Re-shuffle.

Review Comment:
   gather means put all data into one node, right?



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/DistributionSpecHash.java:
##########
@@ -0,0 +1,102 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.properties;
+
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+
+import java.util.List;
+
+/**
+ * Describe hash distribution.
+ */
+public class DistributionSpecHash extends DistributionSpec {
+
+    private final List<SlotReference> shuffledColumns;
+    private final ShuffleType shuffleType;
+    private PropertyInfo propertyInfo;
+
+    public DistributionSpecHash(List<SlotReference> shuffledColumns, ShuffleType shuffleType) {
+        Preconditions.checkState(!shuffledColumns.isEmpty());
+        this.shuffledColumns = shuffledColumns;
+        this.shuffleType = shuffleType;
+    }
+
+    public DistributionSpecHash(List<SlotReference> shuffledColumns, ShuffleType shuffleType,
+            PropertyInfo propertyInfo) {
+        Preconditions.checkState(!shuffledColumns.isEmpty());
+        this.shuffledColumns = shuffledColumns;
+        this.shuffleType = shuffleType;
+        this.propertyInfo = propertyInfo;
+    }
+
+    public List<SlotReference> getShuffledColumns() {
+        return shuffledColumns;
+    }
+
+    public ShuffleType getShuffleType() {
+        return shuffleType;
+    }
+
+    public PropertyInfo getPropertyInfo() {
+        return propertyInfo;
+    }
+
+    @Override
+    public boolean meet(DistributionSpec other) {
+        if (other instanceof DistributionSpecAny) {
+            return true;
+        }
+
+        if (!(other instanceof DistributionSpecHash)) {
+            return false;
+        }
+
+        // DistributionSpecHash spec = (DistributionSpecHash) other;
+        return false;
+    }
+
+    /**
+     * information of hash distribution.
+     * tableId relate with {@link org.apache.doris.catalog.Table}
+     */
+    public static class PropertyInfo {
+        public List<Long> partitionIds = Lists.newArrayList();
+        // Nullable columns generated by outer join
+        public List<SlotReference> nullableCols = Lists.newArrayList();
+        public boolean isReplicate = false;
+        public long tableId = -1;
+
+        public boolean isSingleton() {
+            return partitionIds.size() == 1;
+        }
+    }
+
+    /**
+     * Enums for concrete shuffle type.
+     */
+    public enum ShuffleType {

Review Comment:
   please add some comment to describe each type



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/RequestPropertyDeriver.java:
##########
@@ -0,0 +1,127 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.properties;
+
+import org.apache.doris.common.Pair;
+import org.apache.doris.nereids.PlanContext;
+import org.apache.doris.nereids.jobs.JobContext;
+import org.apache.doris.nereids.memo.GroupExpression;
+import org.apache.doris.nereids.properties.DistributionSpecHash.ShuffleType;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.trees.plans.physical.PhysicalHashJoin;
+import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor;
+import org.apache.doris.nereids.util.JoinUtils;
+import org.apache.doris.nereids.util.Utils;
+
+import com.google.common.collect.Lists;
+
+import java.util.List;
+
+/**
+ * Used for parent property drive.
+ */
+public class RequestPropertyDeriver extends PlanVisitor<Void, PlanContext> {
+    /*
+     * requestPropertyFromParent
+     *             │
+     *             ▼
+     *          curNode (current plan node in current CostAndEnforcerJob)
+     *             │
+     *             ▼
+     * requestPropertyToChildren
+     */
+    PhysicalProperties requestPropertyFromParent;
+    List<List<PhysicalProperties>> requestPropertyToChildren;

Review Comment:
   private?



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/DistributionSpecAny.java:
##########
@@ -18,8 +18,16 @@
 package org.apache.doris.nereids.properties;
 
 /**
- * Describe random distribution.
+ * Data can be anywhere on the segments (required only).
  */
-public class RandomDistributionDesc extends DistributionSpec {
+public class DistributionSpecAny extends DistributionSpec {
 
+    public DistributionSpecAny() {
+        super();
+    }
+
+    @Override
+    public boolean meet(DistributionSpec other) {
+        return other instanceof DistributionSpecAny;

Review Comment:
   according to super's function's comment: Whether other `DistributionSpec` is satisfied the current `DistributionSpec`.
   i think this function should always return true.



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/DistributionSpec.java:
##########
@@ -21,52 +21,38 @@
 import org.apache.doris.nereids.memo.GroupExpression;
 import org.apache.doris.nereids.trees.plans.GroupPlan;
 import org.apache.doris.nereids.trees.plans.physical.PhysicalDistribution;
-import org.apache.doris.planner.DataPartition;
 
 import com.google.common.collect.Lists;
 
 /**
  * Spec of data distribution.
+ * GPORCA has more type in CDistributionSpec.
  */
-public class DistributionSpec {
-
-    private DataPartition dataPartition;
-
-    // TODO: why exist?
-    public DistributionSpec() {
-    }
-
-    public DistributionSpec(DataPartition dataPartition) {
-        this.dataPartition = dataPartition;
-    }
-
+public abstract class DistributionSpec {
     /**
      * TODO: need read ORCA.
      * Whether other `DistributionSpec` is satisfied the current `DistributionSpec`.
      *
      * @param other another DistributionSpec.
      */
-    public boolean meet(DistributionSpec other) {
-        return false;
-    }
-
-    public DataPartition getDataPartition() {
-        return dataPartition;
-    }
-
-    public void setDataPartition(DataPartition dataPartition) {
-        this.dataPartition = dataPartition;
-    }
+    public abstract boolean meet(DistributionSpec other);

Review Comment:
   this function name is confused. can not know it present whether this meet other or other meet this.
   



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/DistributionSpec.java:
##########
@@ -21,52 +21,38 @@
 import org.apache.doris.nereids.memo.GroupExpression;
 import org.apache.doris.nereids.trees.plans.GroupPlan;
 import org.apache.doris.nereids.trees.plans.physical.PhysicalDistribution;
-import org.apache.doris.planner.DataPartition;
 
 import com.google.common.collect.Lists;
 
 /**
  * Spec of data distribution.
+ * GPORCA has more type in CDistributionSpec.
  */
-public class DistributionSpec {
-
-    private DataPartition dataPartition;
-
-    // TODO: why exist?
-    public DistributionSpec() {
-    }
-
-    public DistributionSpec(DataPartition dataPartition) {
-        this.dataPartition = dataPartition;
-    }
-
+public abstract class DistributionSpec {

Review Comment:
   add Developing annotion in all developging class comment



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/DistributionSpecHash.java:
##########
@@ -0,0 +1,102 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.properties;
+
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+
+import java.util.List;
+
+/**
+ * Describe hash distribution.
+ */
+public class DistributionSpecHash extends DistributionSpec {
+
+    private final List<SlotReference> shuffledColumns;
+    private final ShuffleType shuffleType;
+    private PropertyInfo propertyInfo;
+
+    public DistributionSpecHash(List<SlotReference> shuffledColumns, ShuffleType shuffleType) {
+        Preconditions.checkState(!shuffledColumns.isEmpty());
+        this.shuffledColumns = shuffledColumns;
+        this.shuffleType = shuffleType;
+    }
+
+    public DistributionSpecHash(List<SlotReference> shuffledColumns, ShuffleType shuffleType,
+            PropertyInfo propertyInfo) {
+        Preconditions.checkState(!shuffledColumns.isEmpty());
+        this.shuffledColumns = shuffledColumns;
+        this.shuffleType = shuffleType;
+        this.propertyInfo = propertyInfo;
+    }
+
+    public List<SlotReference> getShuffledColumns() {
+        return shuffledColumns;
+    }
+
+    public ShuffleType getShuffleType() {
+        return shuffleType;
+    }
+
+    public PropertyInfo getPropertyInfo() {
+        return propertyInfo;
+    }
+
+    @Override
+    public boolean meet(DistributionSpec other) {
+        if (other instanceof DistributionSpecAny) {
+            return true;
+        }
+
+        if (!(other instanceof DistributionSpecHash)) {
+            return false;
+        }
+
+        // DistributionSpecHash spec = (DistributionSpecHash) other;
+        return false;
+    }
+
+    /**
+     * information of hash distribution.
+     * tableId relate with {@link org.apache.doris.catalog.Table}
+     */
+    public static class PropertyInfo {

Review Comment:
   cannot image what this class use for



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/Utils.java:
##########
@@ -51,4 +52,15 @@ public static List<String> qualifiedNameParts(List<String> qualifier, String nam
     public static String qualifiedName(List<String> qualifier, String name) {
         return StringUtils.join(qualifiedNameParts(qualifier, name), ".");
     }
+
+
+    /**
+     * equals for List but ignore order.
+     */
+    public static <E> boolean equalsIgnoreOrder(List<E> one, List<E> other) {
+        if (one.size() != other.size()) {
+            return false;
+        }
+        return new HashSet<>(one).containsAll(other) && new HashSet<>(other).containsAll(one);

Review Comment:
   maybe we could use treeset and then use equals once



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/RequestPropertyDeriver.java:
##########
@@ -0,0 +1,127 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.properties;
+
+import org.apache.doris.common.Pair;
+import org.apache.doris.nereids.PlanContext;
+import org.apache.doris.nereids.jobs.JobContext;
+import org.apache.doris.nereids.memo.GroupExpression;
+import org.apache.doris.nereids.properties.DistributionSpecHash.ShuffleType;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.trees.plans.physical.PhysicalHashJoin;
+import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor;
+import org.apache.doris.nereids.util.JoinUtils;
+import org.apache.doris.nereids.util.Utils;
+
+import com.google.common.collect.Lists;
+
+import java.util.List;
+
+/**
+ * Used for parent property drive.
+ */
+public class RequestPropertyDeriver extends PlanVisitor<Void, PlanContext> {
+    /*
+     * requestPropertyFromParent
+     *             │
+     *             ▼
+     *          curNode (current plan node in current CostAndEnforcerJob)
+     *             │
+     *             ▼
+     * requestPropertyToChildren
+     */
+    PhysicalProperties requestPropertyFromParent;
+    List<List<PhysicalProperties>> requestPropertyToChildren;
+
+    public RequestPropertyDeriver(JobContext context) {
+        this.requestPropertyFromParent = context.getRequiredProperties();
+    }
+
+    public List<List<PhysicalProperties>> getRequiredPropertyListList(GroupExpression groupExpression) {
+        requestPropertyToChildren = Lists.newArrayList();
+        groupExpression.getPlan().accept(this, new PlanContext(groupExpression));
+        return requestPropertyToChildren;
+    }
+
+    @Override
+    public Void visit(Plan plan, PlanContext context) {
+        List<PhysicalProperties> requiredPropertyList = Lists.newArrayList();
+        for (int i = 0; i < context.getGroupExpression().arity(); i++) {
+            requiredPropertyList.add(new PhysicalProperties());
+        }
+        requestPropertyToChildren.add(requiredPropertyList);
+        return null;
+    }
+
+    @Override
+    public Void visitPhysicalHashJoin(PhysicalHashJoin<Plan, Plan> hashJoin, PlanContext context) {
+        // for broadcast join
+        List<PhysicalProperties> propertiesForBroadcast = Lists.newArrayList(
+                new PhysicalProperties(),
+                new PhysicalProperties(new DistributionSpecReplicated())
+        );
+        // for shuffle join
+        Pair<List<SlotReference>, List<SlotReference>> onClauseUsedSlots = JoinUtils.getOnClauseUsedSlots(hashJoin);
+        List<PhysicalProperties> propertiesForShuffle = Lists.newArrayList(
+                new PhysicalProperties(new DistributionSpecHash(onClauseUsedSlots.first, ShuffleType.JOIN)),
+                new PhysicalProperties(new DistributionSpecHash(onClauseUsedSlots.second, ShuffleType.JOIN)));
+
+        if (!JoinUtils.onlyBroadcast(hashJoin)) {
+            requestPropertyToChildren.add(propertiesForShuffle);
+        }
+        if (!JoinUtils.onlyShuffle(hashJoin)) {
+            requestPropertyToChildren.add(propertiesForBroadcast);
+        }
+
+        return null;
+    }
+
+    protected static List<PhysicalProperties> computeShuffleJoinRequiredProperties(

Review Comment:
   this function do not used anymore



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/RequestPropertyDeriver.java:
##########
@@ -0,0 +1,127 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.properties;
+
+import org.apache.doris.common.Pair;
+import org.apache.doris.nereids.PlanContext;
+import org.apache.doris.nereids.jobs.JobContext;
+import org.apache.doris.nereids.memo.GroupExpression;
+import org.apache.doris.nereids.properties.DistributionSpecHash.ShuffleType;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.trees.plans.physical.PhysicalHashJoin;
+import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor;
+import org.apache.doris.nereids.util.JoinUtils;
+import org.apache.doris.nereids.util.Utils;
+
+import com.google.common.collect.Lists;
+
+import java.util.List;
+
+/**
+ * Used for parent property drive.
+ */
+public class RequestPropertyDeriver extends PlanVisitor<Void, PlanContext> {

Review Comment:
   do we need to add other node later?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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