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 0c26f8df4d [refactor](Nereids): move out misunderstanding func from
JoinUtils (#18865)
0c26f8df4d is described below
commit 0c26f8df4d65d1354c34a4a44609be52573b61a8
Author: jakevin <jakevin...@gmail.com>
AuthorDate: Fri Apr 21 14:11:03 2023 +0800
[refactor](Nereids): move out misunderstanding func from JoinUtils (#18865)
---
.../glue/translator/PhysicalPlanTranslator.java | 4 +-
.../nereids/properties/RequestPropertyDeriver.java | 2 +-
.../trees/plans/physical/PhysicalHashJoin.java | 36 ++++++++++++++----
.../org/apache/doris/nereids/util/JoinUtils.java | 44 +---------------------
.../java/org/apache/doris/nereids/util/Utils.java | 29 --------------
.../properties/RequestPropertyDeriverTest.java | 13 ++-----
6 files changed, 37 insertions(+), 91 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 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<PlanFragment, Pla
return leftFragment;
}
- private PlanFragment
constructBucketShuffleJoin(AbstractPhysicalJoin<PhysicalPlan, PhysicalPlan>
physicalHashJoin,
+ private PlanFragment
constructBucketShuffleJoin(PhysicalHashJoin<PhysicalPlan, PhysicalPlan>
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<ExprId>, List<ExprId>> onClauseUsedSlots =
JoinUtils.getOnClauseUsedSlots(physicalHashJoin);
+ Pair<List<ExprId>, List<ExprId>> onClauseUsedSlots =
physicalHashJoin.getHashConjunctsExprIds();
List<ExprId> 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<Void, PlanContext> {
}
private void addShuffleJoinRequestProperty(PhysicalHashJoin<? extends
Plan, ? extends Plan> hashJoin) {
- Pair<List<ExprId>, List<ExprId>> onClauseUsedSlots =
JoinUtils.getOnClauseUsedSlots(hashJoin);
+ Pair<List<ExprId>, List<ExprId>> 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<Expression> hashJoinConjuncts,
List<Expression> 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<ExprId>, List<ExprId>> getHashConjunctsExprIds() {
+ List<ExprId> exprIds1 =
Lists.newArrayListWithCapacity(hashJoinConjuncts.size());
+ List<ExprId> exprIds2 =
Lists.newArrayListWithCapacity(hashJoinConjuncts.size());
+
+ Set<ExprId> leftExprIds = left().getOutputExprIdSet();
+ Set<ExprId> 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, C> R accept(PlanVisitor<R, C> 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<ExprId>, List<ExprId>> getOnClauseUsedSlots(
- AbstractPhysicalJoin<? extends Plan, ? extends Plan> join) {
-
- List<ExprId> exprIds1 =
Lists.newArrayListWithCapacity(join.getHashJoinConjuncts().size());
- List<ExprId> 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<Slot> leftSlot =
ExpressionUtils.extractSlotOrCastOnSlot(equalTo.left());
- Optional<Slot> 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 <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);
- }
-
/**
* 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<Expression>
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<JoinUtils>() {
+ new MockUp<PhysicalHashJoin>() {
@Mock
- Pair<List<ExprId>, List<ExprId>> getOnClauseUsedSlots(
- AbstractPhysicalJoin<? extends Plan, ? extends Plan> join)
{
+ Pair<List<ExprId>, List<ExprId>> 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<JoinUtils>() {
+ new MockUp<PhysicalHashJoin>() {
@Mock
- Pair<List<ExprId>, List<ExprId>> getOnClauseUsedSlots(
- AbstractPhysicalJoin<? extends Plan, ? extends Plan> join)
{
+ Pair<List<ExprId>, List<ExprId>> getHashConjunctsExprIds() {
return Pair.of(Lists.newArrayList(new ExprId(0)),
Lists.newArrayList(new ExprId(1)));
}
};
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
