yujun777 commented on code in PR #68390:
URL: https://github.com/apache/doris/pull/68390#discussion_r4089233444
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVPlanUtil.java:
##########
@@ -1075,13 +1075,32 @@ private static void checkColumnIfChange(MTMV mtmv,
List<ColumnDefinition> analyz
+ "original length is: %s, current length is: %s",
originalColumns.size(), analyzedColumns.size()));
}
- for (int i = 0; i < originalColumns.size(); i++) {
- if (!isTypeLike(originalColumns.get(i).getType(),
analyzedColumns.get(i).getType())) {
+ // Matched by name, not by position. The order of the two lists is
decided by different passes:
+ // the physical schema is laid out when the MV is created, where
MTMVPlanUtil#applyIvmPhysicalKeyLayout
+ // puts the final key columns first, and the analysed list comes from
running that same layout again
+ // with the stored key columns as its input. The two agree except for
a chained IVM MV whose base
+ // tables carry row-id columns of their own: the create pass derives
the visible key prefix from the
+ // identity key slots, the analysed one takes it from the stored keys,
and the base tables' row-id
+ // columns end up in a different block. What this check is for is a
base-table change that makes a
+ // column disappear or change type, and where a column sits is not
part of that.
+ Map<String, Column> originalByName = Maps.newHashMap();
+ for (Column column : originalColumns) {
+ originalByName.put(column.getName().toLowerCase(), column);
+ }
+ for (Column analyzedColumn : analyzedColumns) {
+ Column originalColumn =
originalByName.get(analyzedColumn.getName().toLowerCase());
Review Comment:
Fixed: name matching is IVM's again, and a plain MV is matched by position
as it always was. The reason is in the finding -- a plain MV keeps the query as
written, so the two lists differ by name for an MV with column names of its own
-- and the divergence the name match exists for is an IVM one.
`IvmBaselineRebuildTest#testAnMvWithItsOwnColumnNamesIsStillUsable` builds
`CREATE MATERIALIZED VIEW mv (c_dt, c_k1, c_v1) AS SELECT dt, k1, v1` and runs
the check on it.
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/mtmv/MTMVTask.java:
##########
@@ -448,6 +477,16 @@ 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());
+ return Lists.newArrayList(RefreshAttemptType.COMPLETE);
Review Comment:
Fixed by refusing it: a `PARTITIONS` request that may not fall back now
fails, naming COMPLETE, AUTO and PARTITIONS FALLBACK, and the widening stays
only for the forms whose scope already carries a whole-MV rebuild. Refreshing
the named partitions would have left the MV in SCHEMA_CHANGE with rows nothing
rebuilt.
`MTMVTaskTest#testAStrictPartitionsRefreshIsRefusedRatherThanWidened` covers
both forms.
--
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]
