yujun777 commented on code in PR #68646:
URL: https://github.com/apache/doris/pull/68646#discussion_r4214316500
##########
fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java:
##########
@@ -373,8 +374,30 @@ 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());
+ // Which columns an operation's effect on a view turns on is the
operation's to say, see
+ // AlterOp#queryJudgedColumnNames, and every clause of the alter
has to name them: a batch that
+ // mixes a dropped column with a type change is decided by neither
-- no query says anything
+ // about a type change -- and stays invalidated the way it was
before the queries were asked at
+ // all. Each of them also has to have reached the table. A schema
change that is not a light one
+ // is applied by a job, which may not have run where this hook
runs: the table still holds the
+ // column the change is about, every query still analyses against
it, and an invalidation
+ // decided on that answer would be about the table from before the
change. What is asked is
+ // whether the change has reached the table, which is the same
fact the re-analysis reads, so
+ // the two answers cannot disagree.
+ boolean judgedByQuery = alterOps.stream().allMatch(op ->
!op.queryJudgedColumnNames().isEmpty()
+ && op.hasReachedTheTable(olapTable));
+ // The names of those columns go to the hook rather than a
verdict: what the judgement is about
+ // is the column, and the hook holds the query's answer against it
-- both while asking, in case
+ // the query can reach the name some other way now, and once it
has answered, in case the table
+ // is no longer the one that answered. See MTMVRelationManager.
+ MTMVHookService.QueryJudgedChange queryJudgedChange = judgedByQuery
+ ? new MTMVHookService.QueryJudgedChange(
+
alterOps.stream().map(AlterOp::queryJudgedColumnNames).flatMap(Set::stream)
+ .collect(Collectors.toSet()),
+ () -> alterOps.stream().allMatch(op ->
op.hasReachedTheTable(olapTable)))
+ : null;
+ Env.getCurrentEnv().getMtmvService().alterTable(oldBaseTableInfo,
newBaseTableInfo,
Review Comment:
Not fixed in 45a1894dfe4 -- recorded instead, with the options and their
costs: #68767.
Two things came out of looking at it that are worth having in the record
here: the interval is between the base table's write lock releasing (where
`modifyTableLightSchemaChange` has already published the column,
`SchemaChangeHandler.java:2700`) and this hook invalidating, which is a
locking/atomicity question rather than a part of the query judgement this PR is
about; and observers carry the same interval through the journal, since the
light change is journaled before the invalidation that follows it, which the
smallest of the three options in the issue would not close either.
--
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]
