morrySnow commented on code in PR #13681:
URL: https://github.com/apache/doris/pull/13681#discussion_r1009140690
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/JoinUtils.java:
##########
@@ -285,28 +286,28 @@ public static boolean
couldColocateJoin(DistributionSpecHash leftHashSpec, Distr
}
/**
- * replace hashJoinConjuncts by using slots map.
- * TODO: just support `col1=col2`
+ * replace JoinConjuncts by using slots map.
+ * TODO: just support `col1 [cmp = > < ...] col2`
*/
- public static List<Expression> replaceHashConjuncts(List<Expression>
hashJoinConjuncts,
+ public static List<Expression> replaceJoinConjuncts(List<Expression>
hashJoinConjuncts,
Map<Slot, Slot> replaceMaps) {
List<Expression> newHashJoinConjuncts = Lists.newArrayList();
for (Expression hashJoinConjunct : hashJoinConjuncts) {
- if (!(hashJoinConjunct instanceof EqualTo)) {
+ if (!(hashJoinConjunct instanceof ComparisonPredicate)) {
return null;
}
- EqualTo equalTo = (EqualTo) hashJoinConjunct;
- if (!(equalTo.left() instanceof Slot) || !(equalTo.right()
instanceof Slot)) {
+ ComparisonPredicate cmp = (ComparisonPredicate) hashJoinConjunct;
+ if (!(cmp.left() instanceof Slot) || !(cmp.right() instanceof
Slot)) {
Review Comment:
when we process other conditions on join. we could meet the expression such
as `a.c1 + a.c2 > b.c3`. So we cannot process other join condition with this
constraint
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/InnerJoinLAsscom.java:
##########
@@ -93,13 +97,13 @@ public static boolean checkReorder(LogicalJoin<? extends
Plan, GroupPlan> topJoi
}
/**
- * Split HashCondition into two part.
+ * Split onCondition into two part.
*/
- public static Map<Boolean, List<Expression>>
splitHashConjuncts(List<Expression> topHashConjuncts,
- LogicalJoin<GroupPlan, GroupPlan> bottomJoin) {
+ private static Map<Boolean, List<Expression>>
splitConjuncts(List<Expression> topConjuncts,
+ LogicalJoin<GroupPlan, GroupPlan> bottomJoin, List<Expression>
bottomConjuncts) {
// top: (A B)(error) (A C) (B C) (A B C)
// Split topJoin hashCondition to two part according to include B.
- Map<Boolean, List<Expression>> splitOn = topHashConjuncts.stream()
+ Map<Boolean, List<Expression>> splitOn = topConjuncts.stream()
.collect(Collectors.partitioningBy(topHashOn -> {
Set<Slot> usedSlot =
topHashOn.collect(Slot.class::isInstance);
Review Comment:
nit: could use getInputSlots function in Expression
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/OuterJoinLAsscomProject.java:
##########
@@ -89,20 +87,34 @@ public Rule build() {
Set<ExprId> bExprIdSet =
InnerJoinLAsscomProject.getExprIdSetForB(bottomJoin.right(),
newRightProjects);
- /* ********** split HashConjuncts ********** */
- Map<Boolean, List<Expression>> splitOn =
InnerJoinLAsscomProject.splitHashConjunctsWithAlias(
- topJoin.getHashJoinConjuncts(), bottomJoin,
bExprIdSet);
- List<Expression> newTopHashJoinConjuncts =
splitOn.get(true);
+ /* ********** split Conjuncts ********** */
+ Map<Boolean, List<Expression>> splitHashJoinConjuncts
+ = InnerJoinLAsscomProject.splitConjunctsWithAlias(
+ topJoin.getHashJoinConjuncts(), bottomJoin,
bottomJoin.getHashJoinConjuncts(), bExprIdSet);
+ List<Expression> newTopHashJoinConjuncts =
splitHashJoinConjuncts.get(true);
+
Preconditions.checkState(!newTopHashJoinConjuncts.isEmpty(),
+ "LAsscom newTopHashJoinConjuncts join can't
empty");
if (topJoin.getJoinType() != bottomJoin.getJoinType()
&& newTopHashJoinConjuncts.size() !=
bottomJoin.getHashJoinConjuncts().size()) {
return null;
}
- List<Expression> newBottomHashJoinConjuncts =
splitOn.get(false);
+ List<Expression> newBottomHashJoinConjuncts =
splitHashJoinConjuncts.get(false);
if (newBottomHashJoinConjuncts.size() == 0) {
return null;
}
- /* ********** replace HashConjuncts by projects **********
*/
+ Map<Boolean, List<Expression>> splitOtherJoinConjuncts
+ = InnerJoinLAsscomProject.splitConjunctsWithAlias(
+ topJoin.getOtherJoinConjuncts(), bottomJoin,
bottomJoin.getOtherJoinConjuncts(),
+ bExprIdSet);
+ List<Expression> newTopOtherJoinConjuncts =
splitOtherJoinConjuncts.get(true);
+ if (topJoin.getJoinType() != bottomJoin.getJoinType()
+ && newTopOtherJoinConjuncts.size() !=
bottomJoin.getOtherJoinConjuncts().size()) {
+ return null;
Review Comment:
add comment to explain why~
##########
fe/fe-core/src/test/java/org/apache/doris/nereids/rules/exploration/join/InnerJoinLAsscomProjectTest.java:
##########
@@ -127,4 +132,44 @@ void testAliasTopMultiHashJoin() {
).when(join -> join.getHashJoinConjuncts().size() == 2)
);
}
+
+ @Test
+ public void testHashAndOther() {
+ List<Expression> bottomHashJoinConjunct = ImmutableList.of(
Review Comment:
we need add some complicate expressions in other condition test.
```
a.c1 + a.c2 > b.c1
a.c1 + b.c1 > 0
substring(a.c1, 5, 10) != '123456'
...
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/InnerJoinLAsscom.java:
##########
@@ -93,13 +97,13 @@ public static boolean checkReorder(LogicalJoin<? extends
Plan, GroupPlan> topJoi
}
/**
- * Split HashCondition into two part.
+ * Split onCondition into two part.
Review Comment:
add more comment, to explain which expression will split into true part and
which expression will split into the false part
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
