yujun777 commented on PR #59972: URL: https://github.com/apache/doris/pull/59972#issuecomment-5250555878
Thanks for the PR. I reviewed the full pipeline (`calculatePartitionMappings` → `MTMVRefreshContext.buildContext` → `getMTMVCanRewritePartitions` / `PartitionCompensator` → `getBaseVersions`) and the unit tests cover the individual pieces well (generator filtering, expander, version fetch, cache coverage). However, I'd suggest adding an end-to-end regression test before merging, because the correctness of the rewrite path now relies on several "conservative degradation" assumptions that are only exercised together in a real query: 1. **EXPR-type MV (e.g. monthly `date_trunc`) on a daily-partitioned base table + a partition-pruned query** — verifies the expanded mapping keeps the full per-MV-partition mapping (so `isSyncWithPartitions` stays correct) while `getMTMVCanRewritePartitions` still returns the right MV partitions and the compensator produces the correct union/remove sets. This is the main scenario the optimization targets, and there is currently no end-to-end case for it. 2. **Multi-pct-table MV where the query references only a subset of the pct tables** — the mapping is filtered per table, and the unreferenced table falls back to a full mapping; an end-to-end test would pin down that rewrite still returns correct results. 3. **Misaligned base partition spanning an MV boundary** (e.g. a base partition crossing a month boundary) — with this PR it is excluded from the expanded filter and the query falls back to the base table for that data. A test asserting the query result stays correct (and, if desired, that the MV is still usable for the aligned partitions) would guard this behavior change. `PartitionCompensatorTest` was only updated for the signature change, and the new unit tests use mocks for the context/mapping pieces. A case under `mtmv_p0` (or an extension of the existing compensator tests with a real filtered mapping) would close the gap between the unit-level coverage and the end-to-end rewrite behavior. -- 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]
