yujun777 commented on code in PR #68646:
URL: https://github.com/apache/doris/pull/68646#discussion_r4219158655
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRelationManager.java:
##########
@@ -349,34 +360,221 @@ 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 places 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. 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;
+ }
+ if (reachesAnyColumnOfASubquery(plan, names, baseTableInfo)) {
+ return true;
+ }
+ return reachesAnyColumnAcrossScopes(plan, lineage, names,
baseTableInfo);
Review Comment:
Not covered in 5dcbddd3e14, and I would rather leave it to a change of its
own than add the rules for it here.
A view's definition is now written with every clause's references qualified,
so what is left is the definitions written before this change -- a stored
`QUALIFY flag = 1`, which keeps the name and lets a later DROP take the column
it named away from it. Four things have to line up for that to cost anything: a
`QUALIFY` in the view's query, a base column referenced from it, that column
dropped or renamed, and another answer for the name left in the query (the
view's own output alias of that name, or a same-named column of another table
in the same scope). Without the last one the re-analysis refuses the query and
the view is invalidated as it should be. Measured on a view built before the
change: after the DROP the view is NORMAL, in sync, holding no rows, while the
same query returns `(1,1)`.
What it does not do today is return wrong rows through the transparent
rewrite: a view with a `QUALIFY` is not selected for rewrite at all (`EXPLAIN`
reports `View struct info is invalid` for it, where the same query without the
`QUALIFY` is chosen), so the exposure is the view's own contents and the views
built on it -- which is what makes me willing to leave it rather than
approximate again inside this change.
Closing it needs two rules, written down rather than left as "not fixed":
- the check reads a subquery's own scope and would have to read the view's
own the same way: a changed name read there as a value the view names itself
(the alias `flag` above) is one the change could have taken from a column,
which is what the precedence in `QUALIFY` says -- the column answers for the
name while it is there, and the alias answers for it once it is not;
- and `LineageInfoExtractor`'s indirect visitor would have to read a
`LogicalQualify` the way it reads a `LogicalFilter`, so that a changed column
read inside a `QUALIFY` predicate is a dependency in the other direction, where
a name is given to the table rather than taken away from it.
--
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]
