yujun777 commented on code in PR #68390:
URL: https://github.com/apache/doris/pull/68390#discussion_r4083024494
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRelationManager.java:
##########
@@ -359,16 +362,48 @@ public void alterTable(BaseTableInfo oldTableInfo,
Optional<BaseTableInfo> newTa
// when replace, need deal two table
if (isReplace) {
// REPLACE TABLE already invalidates the IVM baseline explicitly,
see Alter#processReplaceTable
- processBaseTableChange(newTableInfo.get(), "The base table has
been updated:", false);
+ processBaseTableChange(newTableInfo.get(), "The base table has
been updated:", false, false);
}
- // A RENAME leaves every column alone, and the failure it does cause
-- the MV query still
- // spells the old name -- is already reported by the refresh itself
(MTMVTask#run resolves
- // the base tables from the query before it ever looks at the
baseline). Invalidating here
- // would only leave a stale flag behind: rename the table back and the
query is analyzable
- // again, yet every strict INCREMENTAL refresh would stay rejected
until a COMPLETE one ran.
boolean renamed = !isReplace && newTableInfo.isPresent()
&& !Objects.equals(oldTableInfo.getTableName(),
newTableInfo.get().getTableName());
- processBaseTableChange(oldTableInfo, "The base table has been
updated:", !renamed);
+ // The invalidation runs first, while the dependencies are still
registered under the name the
+ // rename is leaving: moving them first would make this lookup --
which is by the old name -- find
+ // nothing, and the rename would stop invalidating anything at all.
+ processBaseTableChange(oldTableInfo, "The base table has been
updated:", !renamed, renamed);
+ if (renamed) {
+ renameBaseTable(oldTableInfo, newTableInfo.get());
+ }
+ }
+
+ /**
+ * Move a renamed table's entries in the dependency maps to its new name.
+ *
+ * <p>The maps are keyed by {@link BaseTableInfo}, which compares by name,
and an MV keeps the relation
+ * it was created against -- a rename leaves the MV query spelling the old
name, so it no longer
+ * analyzes and the relation is not recomputed. Without this the maps
would keep the old name, and a
+ * metadata-only change to the table under its new name -- a TRUNCATE,
say, which emits no row binlog --
+ * would find no dependent MV to invalidate. Renaming the table back then
restores an analyzable query
+ * whose MV still holds the rows that change removed, and nothing names
the partition that would have
+ * to be rebuilt. Moving the entries is what a rename needs instead of the
invalidation it used to
+ * carry: a rename changes no rows, so there is nothing to rebuild, only a
lookup that has to keep
+ * working.
+ */
+ private void renameBaseTable(BaseTableInfo oldTableInfo, BaseTableInfo
newTableInfo) {
+ moveRelationKey(tableMTMVs, oldTableInfo, newTableInfo);
+ moveRelationKey(tableMTMVsOneLevelAndFromView, oldTableInfo,
newTableInfo);
+ }
+
+ private void moveRelationKey(Map<BaseTableInfo, Set<BaseTableInfo>> map,
+ BaseTableInfo oldTableInfo, BaseTableInfo newTableInfo) {
+ Set<BaseTableInfo> dependents = map.get(oldTableInfo);
+ if (CollectionUtils.isEmpty(dependents)) {
+ return;
+ }
+ // Registered under the new name before the old one is dropped: a
concurrent base-table change
+ // either still finds the old name or already finds the new one, never
neither. Merged rather than
+ // replaced, because a table dropped and re-created under this name
registers its own dependents.
+ map.computeIfAbsent(newTableInfo, key ->
Sets.newConcurrentHashSet()).addAll(dependents);
Review Comment:
Same: the rename no longer skips the invalidation, so a relation captured
before it cannot outlive it -- the state change is what stops the stale
relation from being published as current.
`IvmBaselineRebuildTest#testRenameTableBackStillRequiresAWholeMvRefresh` pins
that the state survives a rename back, which is the cost accepted for it.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java:
##########
@@ -449,8 +475,11 @@ public void alterMvProperties(AlterMTMV alterMTMV, boolean
isReplay) {
this.schemaChangeVersion++;
this.refreshSnapshot = new MTMVRefreshSnapshot();
}
- if (requireCompleteBaselineRebuild) {
- ivmInfo.requireCompleteBaselineRebuild();
+ if (requireCompleteBaselineRebuild && !isReplay) {
+ // Every partition has to be rebuilt, including the ones this
MV does not have yet, so the
+ // MV goes into the state that says exactly that. Journaled on
its own record, ahead of the
+ // property change below; a replay applies both in that order.
+ invalidateWholeMv("The MV's refresh baseline changed with its
properties");
Review Comment:
Implemented. `MTMV#invalidateWholeMv` applies the change and submits the
record, handing the journal write back to the caller; `alterMvProperties`
awaits it after `writeMvUnlock()`, next to the property record it already
awaited there. The order between the two records is the enqueue order under the
same lock. The callers that hold no MV lock await immediately, which is what
they did before.
--
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]
