yujun777 commented on PR #68648: URL: https://github.com/apache/doris/pull/68648#issuecomment-6169953726
The cloud_p0 failure of `nereids_rules_p0/mv/partition_union_rewrite/partition_mv_rewrite` (line 391, `expect contains '.mv_10000 chose'`) was a regression from this PR. Fixed in 206d9a32b4a; the detail, because the mechanism is not obvious from the assertion: **Reproduction.** On a cloud cluster created with `regression-test/pipeline/cloud_p0/conf/fe_custom.conf`'s settings (`enable_auto_collect_statistics=false`, `enable_cloud_snapshot_version=true`, ...) and over the suite's own tables, `mv_10000` (a `PARTITION BY(l_shipdate)` MV over a month-partitioned base, `partition_sync_limit = 2 DAY`) is a rewrite candidate but its plan scan reads `partitions(0/1)` with `cost=0`, so `MaterializedViewRewriteSuccessButNotChose: .mv_10000 not chose`. Swapping only the FE jar for one built from master (`1f5d8bcb350`, the base of this branch) and running the identical SQL gives `.mv_10000 chose` -- so this branch changed it. **Mechanism.** `PartitionCompensator` takes the MV partitions a plan uses out of the union -- and reads the query's partitions from the base table -- when the query reads a base partition no MV partition of the plan is named from, which is exactly what the 2 DAY window leaves (`p1`, `p2`). Whether that is needed decides the answer (an MV partition holding such a partition's rows would count them twice), and the guard asked whether such a partition was *missing* from the MV's refresh snapshot. That reading only held while the snapshot named more than a refresh reads; this PR makes the snapshot name exactly what the refresh reads, so a windowed MV partition records just the partitions the window keeps and the guard fired for every such MV whose window had left a queried base partition out. **Fix.** The guard now asks whether an uncovered partition is one the MV's snapshots record for the plan's MV partitions -- one a refresh read into them (`Sets.intersection`, not `Sets.difference`). A partition the window left out and never refreshed into the MV is left to the union's own base branch, and the MV keeps answering for the partitions it does hold (so the double counting this guard exists for -- `mtmv_p0/test_mtmv_base_partition_read_scope`'s `sync_window_narrowed`, whose MV snapshots do record the dropped partition -- still takes them out). **Measured.** With the change, the same cluster and data give `.mv_10000 chose` and the suite passes; `mtmv_p0/test_mtmv_base_partition_read_scope`, `test_use_for_rewrite` and `test_multi_pct_union_mtmv` pass; `PartitionCompensatorTest` (13), `MTMVRewriteUtilTest` (19), `MTMVTest` (49), `MTMVPartitionUtilTest` (18) and `UpdateMvByPartitionCommandTest` (15) pass. One observation for the pipeline, not for this PR: the assertion's expected value is computed from `is_partition_statistics_ready(...)`, so with statistics not ready the check degrades to "the MV is a candidate at all" and the failure above does not show. That is why it only appeared in cloud_p0 and not in a single-suite local run. -- 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]
