github-actions[bot] commented on code in PR #61635:
URL: https://github.com/apache/doris/pull/61635#discussion_r2976000927
##########
fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PushDownFilterThroughJoinTest.java:
##########
@@ -226,4 +231,36 @@ private void bothSideToRight(JoinType joinType) {
)
);
}
+
+ @Test
+ public void shouldRewritePreferPushDownProjectInOrFilterToSlot() {
+ Expression preferPushDownProjectExpr = new MatchAny(
Review Comment:
**[Medium] Insufficient test coverage**: This single test only covers one
scenario: a `MatchAny` on the **left** side of an `INNER_JOIN` inside an `OR`
predicate. Consider adding tests for:
1. **Right-side pushdown** -- `PreferPushDownProject` expression referencing
only right-child slots (exercises `childIndexToPushedAlias.get(1)` path,
currently untested).
2. **Both sides simultaneously** -- two `PreferPushDownProject` expressions,
one per side, verifying two separate `LogicalProject` nodes are inserted.
3. **Dedup path** -- the same `PreferPushDownProject` expression appearing
in multiple predicates, verifying `oldExprToNewSlot` cache works correctly.
4. **Cross-side expression (negative case)** -- a `PreferPushDownProject`
whose input slots span both sides, verifying it remains unrewritten.
5. **Non-inner join types** -- verify project insertion works correctly with
`LEFT_OUTER_JOIN` etc., where the filter remains above the join.
6. **Standalone PreferPushDownProject predicate** -- not wrapped in OR,
verifying it gets rewritten and can then be pushed to one side.
##########
fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PushDownFilterThroughJoinTest.java:
##########
@@ -226,4 +231,36 @@ private void bothSideToRight(JoinType joinType) {
)
);
}
+
+ @Test
+ public void shouldRewritePreferPushDownProjectInOrFilterToSlot() {
+ Expression preferPushDownProjectExpr = new MatchAny(
+ new Add(rStudent.getOutput().get(0), Literal.of(1)),
+ Literal.of("abc"));
+ Expression rightSidePredicate = new
GreaterThan(rScore.getOutput().get(2), Literal.of(60));
+ Expression orPredicate = new Or(preferPushDownProjectExpr,
rightSidePredicate);
+
+ LogicalPlan plan = new LogicalPlanBuilder(rStudent)
+ .joinEmptyOn(rScore, JoinType.INNER_JOIN)
+ .filter(orPredicate)
+ .build();
+
+ PlanChecker.from(connectContext, plan)
+ .applyTopDown(PushDownFilterThroughJoin.INSTANCE)
+ .matchesFromRoot(logicalFilter(
+ logicalJoin(
+ logicalProject(logicalOlapScan())
+ .when(project ->
project.getProjects().stream()
+
.filter(Alias.class::isInstance)
+ .map(Alias.class::cast)
+ .map(Alias::child)
+
.anyMatch(PreferPushDownProject.class::isInstance)),
+ logicalOlapScan()))
+ .when(filter -> {
+ Expression rewrittenPredicate =
ImmutableList.copyOf(filter.getConjuncts()).get(0);
+ return rewrittenPredicate instanceof Or
+ &&
rewrittenPredicate.anyMatch(SlotReference.class::isInstance)
+ &&
!rewrittenPredicate.anyMatch(PreferPushDownProject.class::isInstance);
Review Comment:
**[Low] Weak assertion**:
`rewrittenPredicate.anyMatch(SlotReference.class::isInstance)` is trivially
satisfied by `rScore.getOutput().get(2)` (the `SlotReference` inside
`rightSidePredicate`). This assertion would pass even if the
`PreferPushDownProject` expression simply disappeared rather than being
correctly replaced with a new slot.
Consider a stronger assertion, e.g., verifying that the `Or`'s first child
is a `SlotReference` (the replacement slot), or extracting the alias slot from
the project and checking it appears in the predicate:
```java
// e.g., check the Or's left child is a SlotReference that didn't exist
before
return rewrittenPredicate instanceof Or
&& ((Or) rewrittenPredicate).left() instanceof SlotReference
&& !rewrittenPredicate.anyMatch(PreferPushDownProject.class::isInstance);
```
--
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]
