yujun777 commented on code in PR #68646:
URL: https://github.com/apache/doris/pull/68646#discussion_r4140239430
##########
fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java:
##########
@@ -373,8 +373,19 @@ private boolean
processAlterOlapTableInternal(List<AlterOp> alterOps, OlapTable
throw new DdlException("Invalid alter operations: " +
currentAlterOps);
}
if (needChangeMTMVState(alterOps)) {
- Env.getCurrentEnv().getMtmvService()
- .alterTable(oldBaseTableInfo, newBaseTableInfo,
currentAlterOps.hasReplaceTableOp());
+ // Whether the state a dependent MV ends in is decided by
re-analysing its query is the
+ // operation's to say, see AlterOp#needQueryUsabilityCheck -- and
the operation is asked whether
+ // the change it carries has reached the table, because the query
is analysed against the table
+ // as it is now. A schema change that is not a light one is
applied by a job, and where that job
+ // has not run yet the table is the one from before the change:
every query still analyses
+ // against it, and an invalidation decided on that answer would be
about the wrong table. A
+ // change that has not been seen that way keeps invalidating the
MVs that read the table, which
+ // is what it did before the queries were asked at all.
+ boolean judgeStateByQueryUsability =
alterOps.stream().anyMatch(AlterOp::needQueryUsabilityCheck)
Review Comment:
Fixed in 0d62f6ec5bf. The judgement is taken only when every clause of the
alter names the column it is about (`AlterOp#queryJudgedColumnNames`), and each
of those columns also has to have reached the table. A batch that mixes a
dropped column with a type change names columns for neither, so it keeps
invalidating the way it did before the queries were asked at all.
##########
fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java:
##########
@@ -373,8 +373,19 @@ private boolean
processAlterOlapTableInternal(List<AlterOp> alterOps, OlapTable
throw new DdlException("Invalid alter operations: " +
currentAlterOps);
}
if (needChangeMTMVState(alterOps)) {
- Env.getCurrentEnv().getMtmvService()
- .alterTable(oldBaseTableInfo, newBaseTableInfo,
currentAlterOps.hasReplaceTableOp());
+ // Whether the state a dependent MV ends in is decided by
re-analysing its query is the
+ // operation's to say, see AlterOp#needQueryUsabilityCheck -- and
the operation is asked whether
+ // the change it carries has reached the table, because the query
is analysed against the table
+ // as it is now. A schema change that is not a light one is
applied by a job, and where that job
+ // has not run yet the table is the one from before the change:
every query still analyses
+ // against it, and an invalidation decided on that answer would be
about the wrong table. A
+ // change that has not been seen that way keeps invalidating the
MVs that read the table, which
+ // is what it did before the queries were asked at all.
+ boolean judgeStateByQueryUsability =
alterOps.stream().anyMatch(AlterOp::needQueryUsabilityCheck)
+ &&
alterOps.stream().filter(AlterOp::needQueryUsabilityCheck)
+ .allMatch(op -> op.hasReachedTheTable(olapTable));
Review Comment:
Fixed in 0d62f6ec5bf. The hook now carries the change itself
(`MTMVHookService.QueryJudgedChange`) and asks it whether it has reached the
table **again, after** the query has been analysed: a column added back under
the same name in between makes the answer about a table the change did not
leave, and the MV is invalidated.
Worth recording that my first attempt at this was wrong: re-reading whether
the columns are in the table, rather than the change's own answer, invalidated
every add of a column. The regression case added here catches it (adding a name
no query reaches must leave the MV alone).
--
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]
