yujun777 commented on code in PR #68861:
URL: https://github.com/apache/doris/pull/68861#discussion_r4270461209
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewAggregateRule.java:
##########
@@ -588,6 +610,16 @@ private boolean isGroupByEquals(Pair<Plan,
LogicalAggregate<Plan>> queryTopPlanA
LogicalAggregate<Plan> queryAggregate = queryTopPlanAndAggPair.value();
LogicalAggregate<Plan> viewAggregate = viewTopPlanAndAggPair.value();
+ // If the query is a scalar aggregate but the view groups by extra
dimensions, the dimensions
+ // removed by the uniform function dependency below only guarantee at
most one group, not that a
+ // group exists. Eliminating the aggregate into a bare project would
lose the single row a scalar
+ // aggregate must always emit when the filter matches zero rows. Keep
the aggregate by rejecting
+ // this fast path and falling back to the roll up rewrite.
+ if (queryAggregate.getGroupByExpressions().isEmpty()
+ && !viewAggregate.getGroupByExpressions().isEmpty()) {
Review Comment:
Confirmed, and it is this PR that makes the case reachable. Reproduced with
`set enable_sql_cache=false`
(the session default is on, which is why my first pass over these queries
showed no difference): on this
branch `select count(*) from base where k = 9999 having count(*) > 0` against
`select k, count(*) cnt from base group by k having count(*) > 0` returned a
row of `0` with the rewrite
on, while the same query with `enable_materialized_view_rewrite=false`
returned no row. A build without
the guard of this PR returns no row for every one of the combinations below,
so the guard is what lets
the query reach the roll up.
The predicate is lost in the compensation, not in `aggregateRewriteByView`:
the view's own residual
`cnt > 0`, mapped back onto query slots, is the query's `count(*) > 0`, so
`Predicates.compensatePredicates` dropped it as exactly covered by the view
(and
`removePredicatesImpliedByViewResidual` would have dropped it as implied as
well), both of which run
before `rejectUnsafeResidualCompensation` gets to look at it. The rewritten
plan then had no filter
above the rolled up aggregate, and `SUM0` over the empty row set is `0`,
which is a row.
Fixed in `Predicates`: a candidate which constrains an aggregate or a window
output is no longer pruned
as redundant because the view states the same condition. The view enforced
it over the value the view
aggregated, while the query needs it over the value its own aggregate
produces from the view rows, and
the roll up changes that value. Such a predicate now stays in the
compensation, where the existing
`rejectUnsafeResidualCompensation` check sees it and refuses the rewrite.
Validation, with `enable_sql_cache=false`, comparing rewrite on against
`enable_materialized_view_rewrite=false` for `k in {1, 9999}` and `having
count(*) > {0, 5, 19, 20, 21,
100000}`: all 12 answers are identical, including the reported one. A view
which has a `HAVING` above
its aggregate is now refused for these queries the same way it already is
for every other query against
it, so the coverage cost is limited to a scalar query repeating an aggregate
mv's own `HAVING`.
Regression: `agg_scalar_empty_input` now creates `scalar_agg_having_mv`
(`select k, count(*) as cnt from ... group by k having count(*) > 0`) and
asserts
`async_mv_rewrite_fail` / `mv_rewrite_fail` for both the matched and the
empty-input query, together with
the rewrite-off / rewrite-on result pairs which pin the empty result. The
case where the mv has no
`HAVING` of its own and the query `HAVING` stays above the roll up (`having
count(*) > 100000`) still
picks the mv and is unchanged.
--
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]
