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


##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/mtmv/MTMVTask.java:
##########
@@ -855,6 +889,51 @@ private void 
executePartitionBasedRefresh(MTMVRefreshContext context, RefreshMod
                 mtmv.getDatabase().getFullName(), mtmv.getName(), getTaskId());
     }
 
+    /**
+     * Whether a base table of the MV has no stream that can be read, which no 
attempt other than
+     * COMPLETE can work around.
+     *
+     * <p>A base table that cannot be resolved is skipped rather than judged: 
it says nothing about the
+     * streams, and the refresh fails on it for its own reasons -- the attempt 
that runs reports that,
+     * this one only decides which attempt that should be.
+     */
+    private boolean hasUnusableIvmStream() {
+        Database mvDb = (Database) mtmv.getDatabase();
+        if (mvDb == null) {
+            // Nothing to look the streams up in, so there is nothing to 
decide here.
+            return false;
+        }
+        Set<TableNameInfo> excluded = mtmv.getExcludedTriggerTables();
+        for (BaseTableInfo baseTableInfo : relation.getBaseTables()) {

Review Comment:
   Fixed in 77770d78ac1.
   
   Both halves of the diagnosis hold. `getBaseTables()` is the transitive 
closure (`generateMTMVRelation` folds in the relation of every scanned MTMV, 
and `MTMVRelation` documents `mv2` as `baseTables = mv1,t1,t2`), and 
`STREAM_UNSUPPORTED` can only come from `IvmUtil.getIvmStream`, whose only two 
callers -- `IvmDeltaRewriter.createDeltaRewriteState` and 
`IvmFullRefreshMTMV.rewriteScan` -- collect streams from the plan's 
`LogicalOlapScan` nodes. I reproduced it on a cluster before changing anything: 
in the chain `mv2 -> mv1 -> t`, `mv2` owns a stream on `t` that no rewrite 
reads; dropping only that one, `REFRESH mv2 INCREMENTAL` succeeded (so the 
incremental path never needed it), while `REFRESH mv2 AUTO` went to 
`RefreshMode=COMPLETE`, `IvmFallbackReason=STREAM_UNSUPPORTED` and rebuilt the 
whole MV.
   
   The check now iterates `relation.getBaseTablesOneLevelAndFromView()` -- the 
tables in the plan, which is the set `beforeMTMVRefresh` and the 
partition-snapshot generation already use for "the tables this refresh reads". 
I picked it over the normalized plan's scan leaves because it is already the 
task's notion of what a refresh reads and needs no extra plan walk; the leaves 
are a subset of it (PCT tables are derived from the rewritten plan too), so the 
partition attempt's stream reads stay covered. `reconcileIvmStreams` still 
walks the closure: keeping every stream the MV owns healthy is its job, and it 
only runs on COMPLETE.
   
   The narrowing keeps the case the preflight exists for: dropping the stream 
of a table the plan does scan still routes to `COMPLETE`/`STREAM_UNSUPPORTED` 
and recreates it, re-verified on the same cluster.
   
   Coverage: 
`MTMVTaskTest.testBuildAttemptsIgnoresAStreamOnlyTheClosureCarries` -- with the 
check on the closure it gets `[COMPLETE]` instead of `[IVM, PARTITIONS, 
COMPLETE]` -- and 
`regression-test/suites/mtmv_p0/ivm/test_ivm_chained_stream_scope`, which drops 
the downstream MV's stream on the upstream's base table and asserts the refresh 
stays incremental by the task's `RefreshMode` (it is `COMPLETE` without the 
change).
   



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