yujun777 commented on code in PR #68180:
URL: https://github.com/apache/doris/pull/68180#discussion_r4057075400
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java:
##########
@@ -616,26 +622,39 @@ public void invalidateIvmBaseline() {
editLogItem.await();
}
+ /**
+ * Mark the MV partitions that may hold rows read from the changed base
table partitions as needing a
+ * rebuild. When those partitions cannot be determined, the whole MV is
marked instead.
+ */
public void invalidateIvmBaseline(BaseTableInfo baseTableInfo, Map<String,
Long> changedPartitions) {
+ // Computed before the MV lock is taken, not inside it: the mapping
reads the partition items of the
+ // MV and of every PCT table, so it takes those tables' locks, and the
MV lock has to stay a leaf
+ // (nothing may be acquired under it) the way the rest of this class
assumes. The selection does not
+ // need to be atomic with the barrier it produces: the barrier is
recorded under the lock below, and
+ // the names it carries are intersected with the live partition names
when they are consumed
+ // (MTMVTask).
+ Optional<Set<String>> affectedMvPartitions =
selectAffectedMvPartitions(baseTableInfo,
+ changedPartitions);
+ if (affectedMvPartitions.isPresent() &&
affectedMvPartitions.get().isEmpty()) {
+ // No MV partition reads any of the changed base partitions, so
this change cannot leave
+ // anything behind here: there is no barrier to persist, and
skipping the version bump
+ // keeps it from discarding the result of a task that is already
running.
+ LOG.debug("No MV partition is affected by changed base partitions,
mv={}, baseTable={}, "
+ + "changedPartitions={}", name, baseTableInfo,
changedPartitions);
Review Comment:
This needs an EXPR (roll-up) MV partition, and an IVM MV cannot have one, so
the case cannot arise here.
An MV whose partition is a function call gets `MTMVPartitionType.EXPR`
(`LogicalPlanBuilder#visitMTMVPartitionInfo`) and
`MTMVPlanUtil#validateIvmPartition` rejects exactly that for IVM: creating the
MV in this shape fails with `IVM materialized view only supports column
partition`. `enableIvm` is derived from the refresh method at create time
(`CreateMTMVInfo`) and never set afterwards, so an EXPR-partitioned MV cannot
become an IVM MV later either; and when the base table's own partition is a
function, `fillPctInfos` raises the MV's type to EXPR as well, which is
rejected the same way.
What an IVM MV can be is `FOLLOW_BASE_TABLE` -- a plain column partition --
or `SELF_MANAGE`, which already takes the whole-MV path. With a column
partition an MV partition and its base partition have the same column and the
same range, so nothing keeps an MV partition alive once its base partition
leaves the window: `alignMvPartition` drops any MV partition whose desc the
windowed generator no longer produces, and it runs in `syncPartitionsIfNeeded`
before the attempts read anything. A base partition the window has dropped
therefore cannot leave rows behind -- either its MV partition is gone with it,
or partition sync removes it before the next incremental refresh reads an
offset.
So the empty result stays safe for the MVs that can be IVM MVs, and reading
the windowed mapping here is right: its window reflects what the refresh itself
still covers, not what the MV may hold. If there is a shape that does reach the
case, I would like to see it.
--
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]
