github-actions[bot] commented on code in PR #65257:
URL: https://github.com/apache/doris/pull/65257#discussion_r3541804281
##########
fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/eageraggregation/EagerAggRewriterTest.java:
##########
@@ -464,6 +465,27 @@ void testBilateralPushMultiLevelJoin() {
}
}
+ @Test
+ void testEmptyContextDoesNotAddRelationAggregate() {
Review Comment:
The new test only calls `relation.accept(...)` with a hand-built empty
context, so it covers the `visitLogicalRelation` short-circuit but not the
other production path this PR changes: `visitLogicalJoin` now wraps
`context.forOneBranch(...)` with `Optional.ofNullable`, and `forOneBranch`
returns null when a selected join branch has no aggregate functions and no
group keys. Please add a rewrite-level case that reaches that path, for example
`select t1.id1 from t1 cross join t2 group by t1.id1` with eager aggregation
enabled, and assert the branch with no functions/keys is left without an empty
aggregate. That guards the regression through the actual visitor path instead
of only the leaf helper.
--
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]
