yujun777 commented on code in PR #68646:
URL: https://github.com/apache/doris/pull/68646#discussion_r4142246837
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRelationManager.java:
##########
@@ -349,34 +357,118 @@ public void dropTable(Table table) {
// because a dropped table is the one change whose query is gone
beyond doubt. What the two record
// is the same state either way. Unlike a rename it stays an
invalidation: the table is gone for
// good, so the state is not something a later alter can make obsolete.
- processBaseTableChange(new BaseTableInfo(table), "The base table has
been deleted:", false);
+ processBaseTableChange(new BaseTableInfo(table), "The base table has
been deleted:", null);
}
/**
* update mtmv status to `SCHEMA_CHANGE`.
*
* @param isReplace
+ * @param queryJudgedColumns the names the alter gives the table or takes
away from it, which leave the
+ * judgement about each MV's state to that MV's
own query, or null when the
+ * alter is not one a query decides. The names
are carried rather than judged
+ * before the call because the judgement is
about them; see
+ * {@code AlterOp#queryJudgedColumnNames} for
which operations name one, and
+ * {@link #invalidateMvUnlessQueryHolds} for
what is asked about it. A rename
+ * of the base table names no column: it is left
to the record below, which
+ * says what the MV that keeps spelling the old
name needs to hear
*/
@Override
- public void alterTable(BaseTableInfo oldTableInfo, Optional<BaseTableInfo>
newTableInfo, boolean isReplace) {
+ public void alterTable(BaseTableInfo oldTableInfo, Optional<BaseTableInfo>
newTableInfo, boolean isReplace,
+ QueryJudgedChange queryJudgedChange) {
// 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:", null);
}
- boolean renamed = !isReplace && newTableInfo.isPresent()
- && !Objects.equals(oldTableInfo.getTableName(),
newTableInfo.get().getTableName());
- // A rename is the one change whose query check is skipped: the MV
query keeps spelling the old
- // name, so it is unanalyzable by construction, and the reason it
would be invalidated with --
- // "the query is no longer analyzable" -- says less than the message
this call records anyway.
- boolean checkQueryUsable = !renamed;
- processBaseTableChange(oldTableInfo, "The base table has been
updated:", checkQueryUsable);
+ processBaseTableChange(oldTableInfo, "The base table has been
updated:", queryJudgedChange);
}
/**
- * An MV's query is only as good as the base table schema it was analyzed
against. Re-analyzing the
- * MV query here (right after the alter was applied) is what detects a
changed column identity:
+ * Whether the query, as it is analysed now, reads a column of any of
these names, and reads it where
+ * the change can reach it.
+ *
+ * <p>There are two such places, and they are the two ways a name is the
change's to answer for. One is
+ * a column of the table the change is about: that is the column this
view's rows were computed from, and
+ * the names are matched case-insensitively because a name is what moves.
The other is a column the query
+ * reaches across a scope boundary -- the plan records those on the Apply
that stands for the subquery,
+ * whose correlation slots are the outer columns its right side reads --
because such a name is the
+ * scopes' to answer for rather than the query's: the nearest column to
the reference answers for it, so
+ * a column the change takes away from a scope inside leaves the name to
one outside, and a column it
+ * gives to a scope inside takes the name over, while the query goes on
producing the columns it always
+ * produced out of rows from somewhere else. A name reached with the
qualifier of another table inside
+ * the query's own scope is neither: no later change can move it, so one
to a column it does not name is
+ * one this view's rows do not depend on.
+ */
+ private static boolean reachesAnyColumnOf(Plan plan, BaseTableInfo
baseTableInfo, Set<String> columnNames) {
+ if (plan == null) {
+ // A query whose plan was not kept is one this cannot be answered
about, and "it does" is the
+ // answer that keeps the view safe.
+ return true;
+ }
+ Set<String> names = Sets.newTreeSet(String.CASE_INSENSITIVE_ORDER);
+ names.addAll(columnNames);
+ LineageInfo lineage = LineageInfoExtractor.extractLineageInfo(plan);
+ for (SetMultimap<?, Expression> byType :
lineage.getDirectLineageMap().values()) {
+ if (reachesAnyColumn(byType.values(), names, baseTableInfo)) {
+ return true;
+ }
+ }
+ // The dataset predicates once, not once per output column: the
per-output copy of them the lineage
+ // also offers holds the same expressions for every column the query
produces, and scanning it would
+ // visit each of them once per column.
+ if (reachesAnyColumn(lineage.getDatasetIndirectLineageMap().values(),
names, baseTableInfo)) {
+ return true;
+ }
+ return reachesAnyColumnAcrossScopes(plan, lineage, names,
baseTableInfo);
Review Comment:
Fixed in a34b700d5e0. Measured on the shape in your comment before changing
anything: `outer_t (1, 1)`, `inner_t (x=1)`, the MV built and holding `id=1`;
after `ALTER TABLE inner_t ADD COLUMN flag INT DEFAULT 0` the query returned no
row while the MV stayed NORMAL holding `id=1` with `SyncWithBaseTables=1`,
which is the state the rewrite serves from. With the change it is invalidated.
The scope the subquery became is now read expression by expression, the way
the lineage is read for the view's own columns, so a name answered for by the
subquery's own output is one the change is held against. The Apply's
correlation slots stay as the second place, since a name the query resolves
across a scope boundary is one the scopes answer for whether or not it is read
from the table the change is about.
Worth recording where the boundary is, because I went past it first: reading
the whole analysed plan rather than the subquery scopes also invalidates on
columns nothing reads, since an IVM MV's analysed plan carries a projection of
the base table's own columns (`[__DORIS_ROW_LSN_COL__ AS
__DORIS_IVM_ROW_ID_COL__, dt, k1, v1, spare, __DORIS_COMMIT_TSO_COL__,
__DORIS_ROW_LSN_COL__]`). The suite's "a column the MV does not use must not
invalidate it" case caught it, so the extra read stays inside the subquery.
--
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]
