yujun777 commented on code in PR #68390:
URL: https://github.com/apache/doris/pull/68390#discussion_r4091985938
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRelationManager.java:
##########
@@ -469,28 +482,40 @@ private void processBaseViewChange(BaseTableInfo
baseViewInfo, String msgPrefix)
}
}
+ /**
+ * Puts every MV that reads this base table into {@code SCHEMA_CHANGE}.
+ *
+ * @param checkQueryUsable whether to re-analyze each MV's query first; see
+ * {@link #invalidateMvIfQueryUnusable}
+ */
private void processBaseTableChange(BaseTableInfo baseTableInfo, String
msgPrefix,
- boolean checkIvmQueryUsable) {
+ boolean checkQueryUsable) {
Set<BaseTableInfo> mtmvsByBaseTable =
getMtmvsByBaseTableOneLevelAndFromView(baseTableInfo);
if (CollectionUtils.isEmpty(mtmvsByBaseTable)) {
return;
}
for (BaseTableInfo mtmvInfo : mtmvsByBaseTable) {
- Table mtmv = null;
+ Table mvTable = null;
try {
- mtmv = (Table) MTMVUtil.getTable(mtmvInfo);
+ mvTable = (Table) MTMVUtil.getTable(mtmvInfo);
} catch (AnalysisException e) {
LOG.warn(e);
continue;
}
- if (checkIvmQueryUsable) {
- invalidateIvmBaselineIfQueryUnusable(baseTableInfo, mtmv);
+ if (checkQueryUsable && invalidateMvIfQueryUnusable(baseTableInfo,
mvTable)) {
Review Comment:
The mechanism and the cost are exactly as you describe:
`ensureMTMVQueryUsable` reaches `analyzeQueryInternal`, which calls
`planner.planWithLock(logicalSink, ANY, ExplainLevel.ALL_PLAN)`
(`MTMVPlanUtil:590`), so each non-rename base-table ALTER plans every dependent
MV in full, synchronously. `checkQueryUsable` is `!renamed`, and the drop path
passes false, so those two are the bounded cases. I am keeping it anyway,
because of where the check sits in time rather than in the state machine.
What it buys is a reason that is only observable in that moment. Take a
plain MV that reads column `x`, then `ALTER TABLE t DROP COLUMN x` followed by
`ALTER TABLE t ADD COLUMN x INT DEFAULT 100`. Agreed that both outcomes
invalidate the MV -- the generic record sets the same `SCHEMA_CHANGE`, bumps
`schemaChangeVersion` and drops the snapshot -- so the repair (the next refresh
runs as COMPLETE and re-reads the re-created column) does not depend on the
check. What depends on it is knowing why: while `x` is gone the query genuinely
does not analyze, and that is the only window in which this is visible. Once
the same-name column is back, every check that runs later --
`checkColumnIfChange` and `checkMTMVPartitionInfo` at refresh time, which
compare by name and type, position and type for a plain MV -- passes again, so
a refresh-time diagnosis reports nothing at all. Moving the reason there is
moving it to the one place where this case is invisible; that shape (drop a
column, re-add o
ne with the same name) is the ABA reported as DORIS-28305.
So the widening this revision makes is deliberate: before it,
`invalidateIvmBaselineIfQueryUnusable` returned early unless the MV was IVM, so
for a plain MV the reason was never captured in the only window it exists. I
agree the price is real. If it shows up in profiles, the way out that does not
lose the window is a shallower analysis that still feeds `checkColumnIfChange`
and `checkMTMVPartitionInfo` -- both read the analyzed columns and the
partition info, which the analysed plan already carries -- rather than dropping
the check. That is a change to the shared `MTMVPlanUtil`, so it belongs in its
own change rather than here.
One limitation worth recording while we are on it: the detail is transient.
The `ADD COLUMN` that follows runs this hook again, its analysis succeeds, and
the generic record overwrites the detail (`MTMVStatus#updateStateAndDetail`
replaces it whenever the new state is `SCHEMA_CHANGE`). The state and the
repair survive; the sentence does not.
--
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]
