seawinde commented on code in PR #61345:
URL: https://github.com/apache/doris/pull/61345#discussion_r3263224754
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/Predicates.java:
##########
@@ -332,7 +332,7 @@ private static Map<Expression, ExpressionInfo>
collectResidualCandidates(StructI
queryStructInfo.getSplitPredicate().getResidualPredicateMap().keySet());
Map<Expression, ExpressionInfo> residualCandidates = new
LinkedHashMap<>();
for (Expression expression : expressions) {
- if (expression.anyMatch(AggregateFunction.class::isInstance)) {
+ if (ExpressionUtils.hasNonWindowAggregateFunction(expression)) {
Review Comment:
• Could this keep the previous stricter behavior?
This changed the check from:
`expression.anyMatch(AggregateFunction.class::isInstance)`
to:
`ExpressionUtils.hasNonWindowAggregateFunction(expression)`
The new helper intentionally ignores aggregate functions inside
`WindowExpression`, so a residual predicate containing a window aggregate could
now pass compensation, while it was rejected before.
Unless this PR intentionally supports window residual compensation, this
looks risky for detail-MV rewrite because window predicates have a different
evaluation level from ordinary filters. I
think we should either keep the old check, or explicitly reject window
expressions as well, for example:
`ExpressionUtils.hasNonWindowAggregateFunction(expression) ||
expression.containsType(WindowExpression.class)`
--
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]
