yujun777 commented on code in PR #68646:
URL: https://github.com/apache/doris/pull/68646#discussion_r4140240360
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRelationManager.java:
##########
@@ -502,10 +511,19 @@ private void processBaseTableChange(BaseTableInfo
baseTableInfo, String msgPrefi
LOG.warn(e);
continue;
}
- if (checkQueryUsable && invalidateMvIfQueryUnusable(baseTableInfo,
mvTable)) {
- // Invalidated with the reason, which is the more specific of
the two messages and the one
- // this change is worth recording: the state is the same one
the generic record below
- // would set, so writing it too would only bury the reason.
+ if (checkQueryUsable) {
Review Comment:
Fixed in 0d62f6ec5bf, on the query's lineage. The decision is now whether
the query, as it is analysed, reads a column by the name the change is about,
taken from the analysed plan's lineage -- projections, filters, joins,
aggregation, and through the views in between -- so an alias or a string that
happens to read the same is not a hit, and a column reached inside a view is.
Measured on the shape in the comment (`SELECT o.id FROM outer_t o WHERE
EXISTS (SELECT 1 FROM inner_t i WHERE i.id = o.id AND flag = 1)`): dropping
`inner_t.flag` left the MV in NORMAL holding the row the old column produced,
and the same query then analyses with `flag` reached from `outer_t`; with the
change it invalidates. The plan used is the analysed logical one -- the
physical plan's lineage does not reach the filters, which I had to find the
hard way.
The add of a column is judged the same way, in the other direction: a column
added where the query reaches a name answers for it from then on. What is not
addressed here is the creation-time gap you point at, qualifying slots inside
nested subqueries; until that is fixed this invalidates on the name rather than
on the bound identity, which is worth an issue of its own.
##########
regression-test/suites/mtmv_p0/test_drop_unreferenced_column_mtmv.groovy:
##########
@@ -0,0 +1,134 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+import org.junit.Assert;
+
+/**
+ * Which base-table column changes invalidate a materialized view.
+ *
+ * <p>A dropped column is judged by re-analysing the MV's own query -- but
only where that question has an
+ * answer. The query is analysed against the table as it is when the alter
reaches the MV hook, and unless
+ * the change is a light one it has not been applied yet at that point: it was
submitted as a job, the
+ * column is still there, and every query still analyses. An invalidation
decided on that answer would be
+ * about the table from before the change, which is exactly where a dropped
and re-added column leaves the
+ * ABA the invalidation exists for. So a change that is not in place keeps
invalidating, and the two halves
+ * of this suite pin the two answers:
+ * <ol>
+ * <li>a merge-on-write table, where dropping a value column is a light
change: the MV whose query does
+ * not name the column is left alone, and the MV whose query names it is
invalidated;</li>
+ * <li>a duplicate table, where the same drop is not a light one and a job
does the data rewrite: the
+ * column is out of the table's schema before the hook runs all the
same, so the same answer holds.
+ * What the gate is for is the other side of that -- a change a job has
not applied yet, where the
+ * query would be analysed against the table from before it. There the
MV is invalidated, which is
+ * what every column change did before the queries were asked at
all.</li>
+ * </ol>
+ *
+ * <p>The second half is also where the consequence is observable: the rewrite
reaches the MV on that table
+ * and not on a merge-on-write one, so the state the change records is what is
left to report on the first.
+ * The IVM side of the same change, where the state is what escalates the next
refresh to a whole-MV
+ * COMPLETE, is pinned in the ivm directory.
+ */
+suite("test_drop_unreferenced_column_mtmv", "mtmv") {
+ String dbName = context.config.getDbNameByFile(context.file)
+ String suiteName = "test_drop_unreferenced_column_mtmv"
+
+ // ------------------------------------------------- 1. the change is in
place: the query decides
+ String mowTable = "${suiteName}_mow_table"
+ String mowMv = "${suiteName}_mow_mv"
+
+ sql """drop materialized view if exists ${mowMv}"""
+ sql """drop table if exists ${mowTable}"""
+ sql """
+ CREATE TABLE ${mowTable}
+ (
+ k1 INT NOT NULL,
+ amount BIGINT,
+ spare BIGINT
+ )
+ UNIQUE KEY(k1)
+ DISTRIBUTED BY HASH(k1) BUCKETS 2
+ PROPERTIES ("replication_num" = "1",
"enable_unique_key_merge_on_write" = "true")
+ """
+ sql """INSERT INTO ${mowTable} VALUES (1, 100, 7), (2, 200, 8)"""
+ sql """
+ CREATE MATERIALIZED VIEW ${mowMv}
+ BUILD DEFERRED REFRESH COMPLETE ON MANUAL
+ DISTRIBUTED BY HASH(k1) BUCKETS 2
+ PROPERTIES ("replication_num" = "1")
+ AS SELECT k1, SUM(amount) AS total FROM ${mowTable} GROUP BY k1
+ """
+ sql """REFRESH MATERIALIZED VIEW ${mowMv} COMPLETE"""
+ waitingMTMVTaskFinishedByMvName(mowMv)
+ order_qt_mow_baseline "SELECT k1, total FROM ${mowMv}"
+
+ // A column the query does not name. Dropping it gives this MV nothing to
recompute, so it is not
+ // invalidated: it stays a refresh candidate, and the state is where that
shows.
+ sql """ALTER TABLE ${mowTable} DROP COLUMN spare"""
+ assertEquals("FINISHED", getAlterColumnFinalState("${mowTable}"))
+ order_qt_mow_state_after_unreferenced_drop "select
Name,State,RefreshState,SyncWithBaseTables from
mv_infos('database'='${dbName}') where Name='${mowMv}'"
+
+ // A column the query names. The MV cannot be computed from the table any
more, so it is invalidated.
+ sql """ALTER TABLE ${mowTable} DROP COLUMN amount"""
+ assertEquals("FINISHED", getAlterColumnFinalState("${mowTable}"))
+ order_qt_mow_state_after_referenced_drop "select
Name,State,RefreshState,SyncWithBaseTables from
mv_infos('database'='${dbName}') where Name='${mowMv}'"
+ // Neither column was one the rows the MV holds depended on: the
invalidation above is not about the
+ // data being wrong, it is about the query the data stands for.
+ order_qt_mow_rows_after_both_drops "SELECT k1, total FROM ${mowMv}"
+
+ // ------------------------- 2. the same drop on a table whose data a job
rewrites: same answer
+ String dupTable = "${suiteName}_dup_table"
+ String dupMv = "${suiteName}_dup_mv"
+ String dupQuery = "SELECT k1, SUM(amount) AS total FROM ${dupTable} GROUP
BY k1"
+
+ sql """drop materialized view if exists ${dupMv}"""
+ sql """drop table if exists ${dupTable}"""
+ sql """
+ CREATE TABLE ${dupTable}
+ (
+ k1 INT,
+ amount BIGINT,
+ spare BIGINT
+ )
+ DUPLICATE KEY(k1)
+ DISTRIBUTED BY HASH(k1) BUCKETS 2
+ PROPERTIES ("replication_num" = "1")
Review Comment:
Fixed in 0d62f6ec5bf. You are right, and the FE log says the same: the drop
took `SchemaChangeHandler#modifyTableLightSchemaChange`.
The second half of the suite now creates its table with
`light_schema_change=false`, so it is the job path it describes: the hook runs
before the job has applied the change, the table still holds the column and
every query analyses against it, so nothing can be concluded from the query and
the MV is invalidated the way it was before the queries were asked. It asserts
`SCHEMA_CHANGE` and that the rewrite no longer chooses the MV. The light path
keeps its own assertions in the first half, where the column is out of the
schema before the hook runs.
--
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]
