github-actions[bot] commented on code in PR #65414:
URL: https://github.com/apache/doris/pull/65414#discussion_r3654278394
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializedViewAggregateOnNoneAggregateRule.java:
##########
@@ -91,8 +91,8 @@ protected boolean checkMaterializationPattern(StructInfo
structInfo, CascadesCon
PlanCheckContext scanCheckContext =
PlanCheckContext.of(ImmutableSet.of());
return
structInfo.getTopPlan().accept(StructInfo.SCAN_PLAN_PATTERN_CHECKER,
scanCheckContext)
&& !scanCheckContext.isContainsTopAggregate()
- && !joinCheckContext.isContainsTopLimit() &&
!joinCheckContext.isContainsTopTopN()
- && !joinCheckContext.isContainsTopWindow();
+ && !scanCheckContext.isContainsTopLimit() &&
!scanCheckContext.isContainsTopTopN()
Review Comment:
[P2] Add a test that reaches the scan fallback
None of the current tests executes this changed predicate:
`agg_on_none_agg.groovy` has no Limit/TopN/Window case, and the FE tests
exercise the two visitors separately. A plain scan MV with one of these
operators would not distinguish this patch because `PLAN_PATTERN_CHECKER`
already records the same flag in `joinCheckContext`; the fallback is only
observable when the first checker fails before recording it. Please add a
focused test that proves those preconditions, fails against the base code, and
passes here (with a plain-scan positive control). Otherwise this planner fix
remains unexercised—the current FE coverage report is 0/12 changed lines.
--
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]
