github-actions[bot] commented on code in PR #68648:
URL: https://github.com/apache/doris/pull/68648#discussion_r4237150915


##########
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:
   [P1] Fall back when a recorded base partition leaves the sync window. For 
`LIST(d,region)`, let `p_old={(D-1,US)}` and `p_kept={(D-1,EU),(D+100,EU)}` 
merge into MV partition M under a 2 DAY limit; refresh records both. A day 
later `p_old` is outside the window, `p_kept` still maps to M, and M remains 
grace-valid. With another base partition outside the predicate, 
`Aggregate(d,region,SUM(v)) -> Filter(d=D-1) -> Scan(base[p_old,p_kept])` has 
concrete selected partitions and can use union rewrite. 
`getNeedCompensatePartitions` adds `p_old` to the base branch, but this 
`recorded` check prevents removing M, which still stores its US row. The 
rewritten `UNION ALL(M,base[p_old])` counts that committed group twice. Remove 
M and compensate the full query scope whenever a now-unmapped queried partition 
was already materialized in a selected MV partition.



-- 
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]

Reply via email to