yujun777 commented on code in PR #68648:
URL: https://github.com/apache/doris/pull/68648#discussion_r4370676219


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/RefreshMTMVCommand.java:
##########
@@ -194,12 +202,38 @@ protected LogicalPlan createRefreshCommand(MTMV mtmv, 
StatementContext statement
                 
statementContext.setExcludedTriggerTables(mtmv.getExcludedTriggerTables());
                 return createIvmIncrRefreshManager().buildInsertCommand(mtmv);
             case COMPLETE:
+                
statementContext.setExcludedTriggerTables(mtmv.getExcludedTriggerTables());
+                Set<String> completeRefreshPartitions = 
getCompleteRefreshPartitions(mtmv);
+                Map<TableIf, String> tableWithPartKey = 
getIncrementalTableMap(mtmv);
+                // Pinned before the context is built, the way a refresh task 
pins its inputs before its
+                // own: the mapping, the read predicates and the scan binding 
then all resolve one
+                // snapshot per input, instead of the mapping re-materializing 
the latest one per MV
+                // partition and the scan possibly binding to a different one 
than the mapping read.
+                Map<MvccTableInfo, MvccSnapshot> pinnedSnapshots =
+                        pinBaseTableSnapshots(mtmv, statementContext);
+                MTMVRefreshContext refreshContext =
+                        MTMVRefreshContext.buildContext(mtmv, 
Maps.newHashMap(), pinnedSnapshots);
                 if (mtmv.isIvm()) {
-                    
statementContext.setIvmRewriteContext(Optional.of(IvmRewriteContext.fullExplain(mtmv)));
+                    // The explained refresh covers every MV partition in one 
plan, which is the refresh-all
+                    // first batch a refresh would run: its PCT inputs are 
reset to the base partitions the
+                    // mapping names -- a table it names as empty is read as 
no rows -- and a non-PCT input is
+                    // read in RESET mode. Both are derived the way MTMVTask 
derives them, from the same
+                    // context, so the preview's reads are the reads execution 
makes; see IvmFullRefreshMTMV.
+                    
statementContext.setIvmRewriteContext(Optional.of(IvmRewriteContext.fullExplain(mtmv,
+                            
MTMVPartitionUtil.collectPctResetPartitionIds(mtmv, refreshContext,
+                                    completeRefreshPartitions),
+                            StreamReadMode.RESET)));
                 }
-                
statementContext.setExcludedTriggerTables(mtmv.getExcludedTriggerTables());
-                return UpdateMvByPartitionCommand.from(
-                        mtmv, getCompleteRefreshPartitions(mtmv), 
getIncrementalTableMap(mtmv), statementContext);
+                // Explained through the base partitions the refresh reads for 
these MV partitions, from the
+                // same mapping the refresh records them with. The MV 
partitions' own key ranges are wider
+                // than that read: with a partition_sync_limit window, a base 
partition it leaves out can lie
+                // inside a retained MV partition, and an explain written from 
the ranges alone would show the
+                // expired partition as read -- a read the refresh does not 
make. An IVM MV passes no
+                // predicate scope: its rewrite scopes the read through the 
reset partition ids instead.
+                return UpdateMvByPartitionCommand.from(mtmv, 
completeRefreshPartitions, tableWithPartKey,
+                        statementContext, mtmv.isIvm() ? null
+                                : 
MTMVPartitionUtil.mappedBasePartitions(tableWithPartKey, refreshContext,
+                                        completeRefreshPartitions));

Review Comment:
   Fixed in 1311d47a31c.
   
   \`MTMVPartitionUtil#mappedBasePartitionsByMvPartition\` now keeps the 
mapping keyed by MV partition, and 
\`UpdateMvByPartitionCommand#constructTableWithPredicates\` ORs one term per MV 
partition -- that partition's recorded base partitions, conjoined with the 
partition's own key range for range tables -- instead of unioning the scopes 
for the batch. Execution batches and the COMPLETE explain share that 
construction, so the preview now matches the per-batch reads:
   
   \`\`\`text
   execution January: p_prev AND January
   execution February: p_kept AND February
   COMPLETE EXPLAIN: (p_prev AND January) OR (p_kept AND February)
   \`\`\`
   
   The boundary fixture now asserts the preview: \`range_spill\`'s COMPLETE 
explain must contain \`(ts < '2024-02-01 00:00:00.000')\` and must not contain 
\`2024-02-01 00:00:00.123\`. A second fixture with \`refresh_partition_num=2\` 
puts the same two partitions in one execution batch and expects the \`.100\` 
row in no MV partition, and 
\`UpdateMvByPartitionCommandTest#testEachMvPartitionIsReadFromItsOwnRecordedBasePartitions\`
 asserts both terms.



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