yujun777 commented on code in PR #68390:
URL: https://github.com/apache/doris/pull/68390#discussion_r4083019776
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/mtmv/MTMVTask.java:
##########
@@ -448,6 +462,17 @@ private List<RefreshAttemptType>
buildAttempts(RefreshRequest request, boolean c
if (shouldUseCompleteForInitialIvmRefresh(containsOneRowRelation)) {
return Lists.newArrayList(RefreshAttemptType.COMPLETE);
}
+ // A schema-level invalidation is not a set of dirty partitions: it
means every partition, including
+ // the ones partition sync has not created yet, and no per-partition
requirement can express that.
+ // IVM only -- a non-IVM MV reaches the same effect through its
cleared snapshot, which its own
+ // refresh already depends on.
+ if (mtmv.isIvm() && !request.explicitPartitions
+ && mtmv.getStatus().getState() == MTMVState.SCHEMA_CHANGE) {
+ LOG.info("IVM MV is in SCHEMA_CHANGE, rebuilding the whole MV,
mv={}, taskId={}",
+ mtmv.getName(), getTaskId());
+ recordRebuiltPartitions(request, mtmv.getPartitionNames().size());
+ return Lists.newArrayList(RefreshAttemptType.COMPLETE);
Review Comment:
Not taken, with the evidence in the conversation above: widening
`capturePlanSignature` to every COMPLETE an IVM MV runs makes three suites
write their delta twice (`test_ivm_snapshot`, `test_ivm_bitmap_agg_2`,
`test_ivm_agg_array_1`), because the fresh signature hands the next refresh to
the incremental path instead of the mismatch fallback.
The comment above the condition argued for the widening until `8b0f4d4cf9a`,
which was misleading -- the condition itself never changed. It now records why
it stays narrow and what has to be understood before it can be widened.
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/mtmv/MTMVTask.java:
##########
@@ -312,15 +321,20 @@ public void run() throws JobException {
// refresh fallback: incompatible MV definitions must fail
directly.
ensureQueryUsableIfNeeded(ctx, tableIfs);
RefreshRequest request = resolveRefreshRequest();
- validateIvmBaselineBeforePartitionSync(request);
- List<RefreshAttemptType> attempts = buildAttempts(request,
queryAnalysis.containsOneRowRelation());
try {
syncPartitionsIfNeeded(ctx, tableIfs);
} catch (PartitionPlanningException e) {
throw new JobException(e.getMessage(), e);
}
+ // Partition sync has decided which partitions exist, and nothing
has read a base table yet:
+ // this is the point where an entry and the partition it describes
become the same thing.
+ // Doing it any later would let a partition that sync has just
added be refreshed without an
+ // entry, and an invalidation arriving in between would have
nothing to land on.
+ mtmv.alignPartitionStates(mtmv.getPartitionNames());
Review Comment:
Implemented. The payload carries only the partitions the result published
(`MTMV#publishedPartitionStates(task.getIvmCapturedEpochs())`) rather than the
whole map, and a replay merges those into the live map instead of replacing it,
so an entry another record owns is not dropped by a task result.
--
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]
