github-actions[bot] commented on code in PR #63915:
URL: https://github.com/apache/doris/pull/63915#discussion_r3348022119
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/eageraggregation/EagerAggRewriter.java:
##########
@@ -505,6 +503,28 @@ public Plan visitLogicalAggregate(LogicalAggregate<?
extends Plan> agg, PushDown
@Override
public Plan visitLogicalFilter(LogicalFilter<? extends Plan> filter,
PushDownAggContext context) {
+ if (filter.child() instanceof LogicalRelation) {
+ return genAggregate(filter, context);
+ }
+ if
(filter.getConjuncts().stream().anyMatch(Expression::containsUniqueFunction)) {
+ return genAggregate(filter, context);
+ }
+ List<SlotReference> filterInputSlots = filter.getInputSlots().stream()
+ .map(slot -> (SlotReference) slot)
+ .collect(Collectors.toList());
+ List<SlotReference> childGroupKeys = Stream.concat(
+ context.getGroupKeys().stream(),
+ filterInputSlots.stream())
+ .distinct()
+ .collect(Collectors.toList());
+ PushDownAggContext childContext =
context.withGroupKeys(childGroupKeys);
+ if (!childContext.isValid()) {
Review Comment:
This new filter push-through can expose an unsafe outer-join aggregate
pushdown that was previously blocked by stopping at the filter. For example,
`SELECT a.id, SUM(COALESCE(b.v, 1)) FROM a LEFT JOIN b ON a.id = b.id WHERE
b.flag IS NULL GROUP BY a.id` keeps null-extended rows at the filter and should
contribute `COALESCE(NULL, 1) = 1` for unmatched `a` rows. After this change,
the context can be pushed through the filter into the join; because the
aggregate argument has an input slot from the nullable right side, the
outer-join guard treats it as safe and pushes the aggregate below the join.
Unmatched rows then have no pre-aggregated right row, so the joined aggregate
slot is `NULL` and the top `SUM` loses the contribution. This is the same
semantic class as the CaseWhen/If protection, but the new filter path makes it
reachable for queries that previously aggregated above the filter. Please
either keep the filter as a stop point for these nullable-side null-producing
expressions
or extend the safety check before allowing the child rewrite.
--
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]
