Doris-Breakwater commented on issue #66264:
URL: https://github.com/apache/doris/issues/66264#issuecomment-5127590969
Initial assessment: **valid, high-severity correctness bug**. The repro is
self-contained and the reported `5` versus `1005` result is consistent with the
optimizer selecting a rollup that cannot enforce `id = 'id1'`. This should be
prioritized as a silent wrong-result issue, not treated as a rollup-selection
performance issue. The issue currently has no labels, assignee, milestone, or
linked PR.
### Code-backed findings
- `EliminateGroupByKeyByUniform` is still active on current `master`,
`branch-4.0`, and `branch-4.1`. For a uniform, non-null grouping slot it
removes the slot from `GROUP BY` and emits `any_value(slot)` with a new
`ExprId`. It records only an `ExprId` replacement; it does not retain the
predicate/proof on which uniformity depended.
- The rewrite ordering runs this rule before the later MV/rollup exploration
path. `ConstantPropagation` can subsequently replace the `any_value(id)`
argument with the equality literal, so the output remains constructible even
when the candidate rollup has no `id`.
- The MV framework already recognizes this rule as plan-shape-changing:
`ELIMINATE_GROUP_BY_KEY_BY_UNIFORM` has been in
`PreMaterializedViewRewriter.NEED_PRE_REWRITE_RULE_TYPES` since #55878.
- There is an important branch difference:
- `branch-4.0` and `branch-4.1` default
`pre_materialized_view_rewrite_strategy` to `NOT_IN_RBO` (the 4.0 default was
changed in #56611 for performance reasons). Therefore MV matching normally sees
the post-elimination plan on these branches.
- Current `master` defaults to `TRY_IN_RBO`, so the exact default repro
may be avoided by matching a recorded pre-transformation plan. This does
**not** prove master is unaffected: the same post-elimination CBO path remains
reachable when `NOT_IN_RBO` is configured.
- The normal safeguard is in `AbstractMaterializedViewRule.doRewrite`: every
compensation predicate must be rewritable against the MV scan, and a rollup
without `id` should therefore be rejected. The control query demonstrates that
path working. For the broken query, the output rewrite through
`any_value`/literal means the equality is no longer protecting candidate
selection by the time compensation is checked.
- The concrete missing invariant is after candidate substitution:
`rewriteQueryByViewPreCheck` does not verify that every original predicate used
to establish a uniform grouping key is either implied by the view definition or
still enforced on the rewritten scan. A plan-local proof (`id` is uniform below
the aggregate because of `id = 'id1'`) is being reused after replacing that
child with a rollup where the proving predicate cannot be represented.
The issue's attribution to the interaction between
`EliminateGroupByKeyByUniform` and sync-MV/rollup rewrite is therefore well
supported. The direct investigation point is the handoff from the post-RBO
`StructInfo` predicate set to `compensatePredicates`: compare the broken and
control queries and confirm where `id = 'id1'` ceases to be a required
compensation predicate.
### Information still needed
The supplied DDL/data are sufficient to reproduce the reported 4.0.7
behavior. To distinguish the RBO and CBO paths and confirm the current-master
status, please add:
1. The exact FE build/commit identifier.
2. `SHOW VARIABLES LIKE 'pre_materialized_view_rewrite_strategy'` and `SHOW
VARIABLES LIKE 'disable_nereids_rules'`.
3. Complete `EXPLAIN VERBOSE` output for both queries, including the full
`MaterializedViewRewriteSuccessAndChose` / failure sections and whether they
say RBO or CBO.
4. The same broken-query result and `EXPLAIN VERBOSE` after:
- `SET pre_materialized_view_rewrite_strategy = 'TRY_IN_RBO';`
- disabling `ELIMINATE_GROUP_BY_KEY_BY_UNIFORM` for the session.
A runtime profile is not needed initially because this is an optimizer
semantic error, unless the physical plan differs from the one described.
### Recommended next steps
1. Reproduce on `branch-4.0`, `branch-4.1`, and current `master` with both
`TRY_IN_RBO` and `NOT_IN_RBO`. Master should not be declared unaffected based
only on its different default.
2. Add a regression case next to
`nereids_rules_p0/mv/agg_optimize_when_uniform`: use the supplied AGG_KEYS
table and coarse rollup, assert the result is `5`, and assert that a rollup
lacking `id` is not chosen. Run the case under both rewrite strategies. Keep
the control query where `id` is not selected.
3. Fix correctness at the MV boundary: carry or recover the predicates that
justify uniform-key elimination, and reject any candidate for which those
predicates are neither implied by the view nor successfully rewritten onto the
candidate scan. Rewriting `any_value(id)` to a literal must not count as
enforcing `id = 'id1'` on the measure input.
4. Until a fix is available, the safest session-level mitigation is to add
`ELIMINATE_GROUP_BY_KEY_BY_UNIFORM` to `disable_nereids_rules`. `TRY_IN_RBO` is
a narrower possible workaround on 4.0/4.1, but it should be validated with the
repro and monitored for the performance concern that caused those branches to
default to `NOT_IN_RBO`. Dropping/avoiding a coarse rollup that omits the
filtered column is another operational workaround.
Breakwater-GitHub-Analysis-Slot: slot_619ab1ad6f55
--
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]
