yujun777 commented on PR #68861:
URL: https://github.com/apache/doris/pull/68861#issuecomment-6098017669
Follow up on the p0 failures: the diagnosis in my previous comment was wrong
— they were not cost related.
Reproducing locally with `set global fe_debug=true` (which is how the p0
pipeline runs, and why these suites passed for me locally before and failed on
CI) shows the mv candidate failing outright instead of losing on cost:
MaterializedViewRewriteFail: CBO.internal.<db>.<mv> fail
FailInfo: View struct info is invalid, Materialized view rule exec fail
AnalysisException: AdjustNullable convert slot sum(__count_1)#7 from
not-nullable to nullable
`LogicalAggregate` adjusts the nullability of its own output expressions in
its constructor: for an aggregate without group by,
`AdjustAggregateNullableForEmptySet` forces a `NullableAggregateFunction` to
`alwaysNullable=true`, because a scalar aggregate over an empty input returns
NULL. The roll up rewrite built the slot that the project above the rewritten
aggregate refers to from the `Alias(sum(...))` constructed *before* the
aggregate node existed, so the project referenced a not-nullable slot for an
output the aggregate makes nullable. `AdjustNullable` treats that as an illegal
flip and throws, which drops the whole candidate and leaves the query on the
base table.
This commit fills the query-output -> rewritten-output mapping (and the
`needTopProject` comparison) from `rewrittenAggregate.getOutputExpressions()`,
so the slot can never disagree with the nullability the aggregate node actually
produces. Where the query function guarantees a non-null result for an empty
input (`count` -> 0) but its roll up does not (`sum` -> NULL), the guarantee is
restored with `ifnull` on the project above the aggregate, so the rewritten
plan still returns one row of 0 rather than NULL. A roll up that keeps the
guarantee itself (`count distinct` -> `bitmap_union_count`) needs no
compensation.
A control case confirms the mechanism: `select sum(k2) from t` over `select
k1, sum(k2) from t group by k1` already rewrote fine under `fe_debug=true`,
because its roll up argument is nullable and no flip happens; only the `count`
roll up was affected.
The two commits that had relaxed `mv_rewrite_success` into
`mv_rewrite_success_without_check_chosen` were justified by the wrong cost
diagnosis, so they are dropped and those suites are back to their original form
— this force-pushes `fix-mv-scalar-count-empty` to a single commit.
Validated locally with `set global fe_debug=true`: `mv_ignore_predicate`,
`sum_devide_count`, `testSelectMVWithTableAlias`, `agg_scalar_empty_input`,
`aggregate_with_roll_up`, `aggregate_without_roll_up`, the `dimension` suites,
and `sum_devide_count` / `MVWithAs` under `nereids_syntax_p0` all pass, with no
`AdjustNullable` flip logged.
`nereids_rules_p0.mv.negative.negative_partition_mv_rewrite` is the suite that
carried the failing case: its scalar aggregate over the grouped
`sum/max/min/count/bitmap_union` mv now rewrites and the mv is chosen
(`CBO.internal.<db>.mv_1 chose`), with results identical to
`enable_materialized_view_rewrite=false`. I could not finish that suite end to
end locally: the refresh of that mv takes minutes for a 10-row base table on my
single-backend cluster and exceeds the suite's five-minute wait, and the same
mv definition is just as slow on a build without this change, so the stall is
not introduced here.
--
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]
