yujun777 commented on code in PR #68648:
URL: https://github.com/apache/doris/pull/68648#discussion_r4237189878
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/PartitionCompensator.java:
##########
@@ -144,15 +147,71 @@ public static Pair<Map<BaseTableInfo, Set<String>>,
Map<BaseColInfo, Set<String>
if (allCompensateIsNull) {
return null;
}
+ // A base partition the query reads that no MV partition of this plan
is named from -- one the
+ // partition_sync_limit window left out, say -- is one the MV does not
hold: a partition of the MV reads
+ // a table through the partitions it does name, and the pass above
works from the mapping, which has no
+ // entry for this one. Adding that partition's rows to the union alone
would not be enough, because the
+ // MV partitions this plan uses still hold the rows they hold and the
answer would count them twice.
+ // The partitions the plan uses are taken out and the query's
partitions are read from the base table,
+ // so the answer is whole and counted once. A partition the MV's
snapshots name for a partition this
+ // plan reads is left alone: the MV does hold its rows, and the
validity pass above decides about them.
+ boolean uncoveredFound = false;
+ for (Entry<MTMVRelatedTableIf, Map<String, Set<String>>> mappingEntry
+ : mtmvRelatedTableIfMapMap.entrySet()) {
+ MTMVRelatedTableIf pctTable = mappingEntry.getKey();
+ Set<String> queryUsed =
queryUsedBaseTablePartitionMap.get(pctTable.getFullQualifiers());
+ if (CollectionUtils.isEmpty(queryUsed)) {
+ continue;
+ }
+ Set<String> named = Sets.newHashSet();
+ for (Set<String> names : mappingEntry.getValue().values()) {
+ named.addAll(names);
+ }
+ Set<String> uncovered = Sets.difference(queryUsed, named);
+ if (uncovered.isEmpty()) {
+ continue;
+ }
+ Set<String> recorded = Sets.newHashSet();
+ MTMVRefreshSnapshot refreshSnapshot = mtmv.getRefreshSnapshot();
+ if (refreshSnapshot != null) {
+ for (String mvPartitionName :
rewrittenPlanUsePartitionNameSet) {
+
recorded.addAll(refreshSnapshot.getPctSnapshots(mvPartitionName, new
BaseTableInfo(pctTable)));
+ }
+ }
+ if (!Sets.difference(uncovered, recorded).isEmpty()) {
+ uncoveredFound = true;
+ }
Review Comment:
Fixed in 64a5488493d, along the line you give: the MV partitions the plan
uses are taken out whenever a queried partition is one no MV partition of the
plan is named from, whatever the MV's snapshots say about it. The check I had
added there -- leave the partition alone when the snapshots name that partition
-- is what a narrowed window walks into, since a partition the window has since
dropped was materialized by the earlier refresh and is named by the snapshot
while the mapping no longer names it; the MV branch then contributes its rows
and the base branch adds them again.
Reproduced on your shape, `LIST(d, region)` with `p_old={(D-1, US)}` and
`p_kept={(D-1, EU), (D+100, EU)}` whose window narrows after the refresh that
recorded both: `select d, region, sum(v) ... group by d, region` returned the
`US` row twice with that check in place, and once, matching the base table,
without it. The plan is a `VUNION` with the MV scan in it, so the compensation
branch is genuinely exercised by the fixture.
It is a regression case now (`sync_window_narrowed` in
`mtmv_p0/test_mtmv_base_partition_read_scope`): the MV is refreshed before its
window narrows and the query then covers the widened predicate. It fails with
the check back in place.
`PartitionCompensatorTest`'s two-table case still passes: its snapshots are
empty, so the check never applied there.
--
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]
