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 4b80f64036..e51e5469b7 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 @@ -1932,14 +1932,14 @@ public class PhysicalPlanTranslator extends DefaultPlanVisitor physicalHashJoin, + private PlanFragment constructBucketShuffleJoin(PhysicalHashJoin physicalHashJoin, HashJoinNode hashJoinNode, PlanFragment leftFragment, PlanFragment rightFragment, PlanTranslatorContext context) { // according to left partition to generate right partition expr list DistributionSpecHash leftDistributionSpec = (DistributionSpecHash) physicalHashJoin.left().getPhysicalProperties().getDistributionSpec(); - Pair, List> onClauseUsedSlots = JoinUtils.getOnClauseUsedSlots(physicalHashJoin); + Pair, List> onClauseUsedSlots = physicalHashJoin.getHashConjunctsExprIds(); List rightPartitionExprIds = Lists.newArrayList(leftDistributionSpec.getOrderedShuffledColumns()); for (int i = 0; i < leftDistributionSpec.getOrderedShuffledColumns().size(); i++) { int idx = leftDistributionSpec.getExprIdToEquivalenceSet() diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/RequestPropertyDeriver.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/RequestPropertyDeriver.java index 2ca4fdc169..a05596f7dc 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/RequestPropertyDeriver.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/RequestPropertyDeriver.java @@ -144,7 +144,7 @@ public class RequestPropertyDeriver extends PlanVisitor { } private void addShuffleJoinRequestProperty(PhysicalHashJoin hashJoin) { - Pair, List> onClauseUsedSlots = JoinUtils.getOnClauseUsedSlots(hashJoin); + Pair, List> onClauseUsedSlots = hashJoin.getHashConjunctsExprIds(); // shuffle join addRequestPropertyToChildren( PhysicalProperties.createHash( diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalHashJoin.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalHashJoin.java index 7351ec2e32..26f56249c3 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalHashJoin.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalHashJoin.java @@ -17,9 +17,11 @@ package org.apache.doris.nereids.trees.plans.physical; +import org.apache.doris.common.Pair; import org.apache.doris.nereids.memo.GroupExpression; import org.apache.doris.nereids.properties.LogicalProperties; import org.apache.doris.nereids.properties.PhysicalProperties; +import org.apache.doris.nereids.trees.expressions.ExprId; import org.apache.doris.nereids.trees.expressions.Expression; import org.apache.doris.nereids.trees.expressions.MarkJoinSlotReference; import org.apache.doris.nereids.trees.plans.JoinHint; @@ -35,6 +37,7 @@ import com.google.common.collect.Lists; import java.util.List; import java.util.Optional; +import java.util.Set; /** * Physical hash join plan. @@ -76,13 +79,7 @@ public class PhysicalHashJoin< groupExpression, logicalProperties, leftChild, rightChild); } - /** - * Constructor of PhysicalHashJoinNode. - * - * @param joinType Which join type, left semi join, inner join... - * @param hashJoinConjuncts conjunct list could use for build hash table in hash join - */ - public PhysicalHashJoin( + private PhysicalHashJoin( JoinType joinType, List hashJoinConjuncts, List otherJoinConjuncts, @@ -98,6 +95,31 @@ public class PhysicalHashJoin< groupExpression, logicalProperties, physicalProperties, statistics, leftChild, rightChild); } + /** + * Get all used slots from hashJoinConjuncts of join. + * Return pair of left used slots and right used slots. + */ + public Pair, List> getHashConjunctsExprIds() { + List exprIds1 = Lists.newArrayListWithCapacity(hashJoinConjuncts.size()); + List exprIds2 = Lists.newArrayListWithCapacity(hashJoinConjuncts.size()); + + Set leftExprIds = left().getOutputExprIdSet(); + Set rightExprIds = right().getOutputExprIdSet(); + + for (Expression expr : hashJoinConjuncts) { + expr.getInputSlotExprIds().forEach(exprId -> { + if (leftExprIds.contains(exprId)) { + exprIds1.add(exprId); + } else if (rightExprIds.contains(exprId)) { + exprIds2.add(exprId); + } else { + throw new RuntimeException("Could not generate valid equal on clause slot pairs for join"); + } + }); + } + return Pair.of(exprIds1, exprIds2); + } + @Override public R accept(PlanVisitor visitor, C context) { return visitor.visitPhysicalHashJoin(this, context); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/JoinUtils.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/JoinUtils.java index ea7b435c96..9e31f7467e 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/JoinUtils.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/JoinUtils.java @@ -44,7 +44,6 @@ import com.google.common.collect.Lists; import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -171,47 +170,6 @@ public class JoinUtils { return result; } - /** - * Get all used slots from onClause of join. - * Return pair of left used slots and right used slots. - */ - public static Pair, List> getOnClauseUsedSlots( - AbstractPhysicalJoin join) { - - List exprIds1 = Lists.newArrayListWithCapacity(join.getHashJoinConjuncts().size()); - List exprIds2 = Lists.newArrayListWithCapacity(join.getHashJoinConjuncts().size()); - - JoinSlotCoverageChecker checker = new JoinSlotCoverageChecker( - join.left().getOutputExprIdSet(), - join.right().getOutputExprIdSet()); - - for (Expression expr : join.getHashJoinConjuncts()) { - EqualTo equalTo = (EqualTo) expr; - // TODO: we could meet a = cast(b as xxx) here, need fix normalize join hash equals future - Optional leftSlot = ExpressionUtils.extractSlotOrCastOnSlot(equalTo.left()); - Optional rightSlot = ExpressionUtils.extractSlotOrCastOnSlot(equalTo.right()); - if (!leftSlot.isPresent() || !rightSlot.isPresent()) { - continue; - } - - ExprId leftExprId = leftSlot.get().getExprId(); - ExprId rightExprId = rightSlot.get().getExprId(); - - if (checker.isCoveredByLeftSlots(leftExprId) - && checker.isCoveredByRightSlots(rightExprId)) { - exprIds1.add(leftExprId); - exprIds2.add(rightExprId); - } else if (checker.isCoveredByLeftSlots(rightExprId) - && checker.isCoveredByRightSlots(leftExprId)) { - exprIds1.add(rightExprId); - exprIds2.add(leftExprId); - } else { - throw new RuntimeException("Could not generate valid equal on clause slot pairs for join: " + join); - } - } - return Pair.of(exprIds1, exprIds2); - } - public static boolean shouldNestedLoopJoin(Join join) { return join.getHashJoinConjuncts().isEmpty(); } @@ -221,7 +179,7 @@ public class JoinUtils { } /** - * The left and right child of origin predicates need to be swap sometimes. + * The left and right child of origin predicates need to swap sometimes. * Case A: * select * from t1 join t2 on t2.id=t1.id * The left plan node is t1 and the right plan node is t2. diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/Utils.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/Utils.java index e19b3e6892..04ab5eb395 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/Utils.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/Utils.java @@ -30,7 +30,6 @@ import org.apache.commons.lang3.StringUtils; import java.util.ArrayList; import java.util.Collection; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -122,16 +121,6 @@ public class Utils { return StringUtils.join(qualifiedNameParts(qualifier, name), "."); } - /** - * equals for List but ignore order. - */ - public static boolean equalsIgnoreOrder(List one, List other) { - if (one.size() != other.size()) { - return false; - } - return new HashSet<>(one).containsAll(other) && new HashSet<>(other).containsAll(one); - } - /** * Get sql string for plan. * @@ -158,24 +147,6 @@ public class Utils { return stringBuilder.append(" )").toString(); } - /** - * See if there are correlated columns in a subquery expression. - */ - public static boolean containCorrelatedSlot(List correlatedSlots, Expression expr) { - if (correlatedSlots.isEmpty() || expr == null) { - return false; - } - if (expr instanceof SlotReference) { - return correlatedSlots.contains(expr); - } - for (Expression child : expr.children()) { - if (containCorrelatedSlot(correlatedSlots, child)) { - return true; - } - } - return false; - } - /** * Get the correlated columns that belong to the subquery, * that is, the correlated columns that can be resolved within the subquery. diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/RequestPropertyDeriverTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/RequestPropertyDeriverTest.java index 57dddbb3df..0c9044eb3d 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/RequestPropertyDeriverTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/RequestPropertyDeriverTest.java @@ -30,15 +30,12 @@ import org.apache.doris.nereids.trees.plans.AggPhase; import org.apache.doris.nereids.trees.plans.GroupPlan; import org.apache.doris.nereids.trees.plans.JoinHint; import org.apache.doris.nereids.trees.plans.JoinType; -import org.apache.doris.nereids.trees.plans.Plan; -import org.apache.doris.nereids.trees.plans.physical.AbstractPhysicalJoin; import org.apache.doris.nereids.trees.plans.physical.PhysicalAssertNumRows; import org.apache.doris.nereids.trees.plans.physical.PhysicalHashAggregate; import org.apache.doris.nereids.trees.plans.physical.PhysicalHashJoin; import org.apache.doris.nereids.trees.plans.physical.PhysicalNestedLoopJoin; import org.apache.doris.nereids.types.IntegerType; import org.apache.doris.nereids.util.ExpressionUtils; -import org.apache.doris.nereids.util.JoinUtils; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; @@ -95,10 +92,9 @@ public class RequestPropertyDeriverTest { @Test public void testShuffleHashJoin() { - new MockUp() { + new MockUp() { @Mock - Pair, List> getOnClauseUsedSlots( - AbstractPhysicalJoin join) { + Pair, List> getHashConjunctsExprIds() { return Pair.of(Lists.newArrayList(new ExprId(0)), Lists.newArrayList(new ExprId(1))); } }; @@ -122,10 +118,9 @@ public class RequestPropertyDeriverTest { @Test public void testShuffleOrBroadcastHashJoin() { - new MockUp() { + new MockUp() { @Mock - Pair, List> getOnClauseUsedSlots( - AbstractPhysicalJoin join) { + Pair, List> getHashConjunctsExprIds() { return Pair.of(Lists.newArrayList(new ExprId(0)), Lists.newArrayList(new ExprId(1))); } };