Doris-Breakwater commented on issue #68767: URL: https://github.com/apache/doris/issues/68767#issuecomment-6051754595
Breakwater-GitHub-Analysis-Slot: slot_2c2a6eb07530 The schema-publication / MTMV-invalidation interval is supported by the source. I recommend prioritizing this as a query-correctness investigation, with an important distinction: the interval and missing schema check are verified statically; an actual stale rewrite and wrong result still need a deterministic concurrent test. I inspected public master at `81556a1b5ef5d52412a8380e505238bc744f46a8` and the current head of [PR #68646](https://github.com/apache/doris/pull/68646), `45a1894dfe406f611d1cb00b1e00daea38ed2681`. The PR is currently open and unmerged. This issue is open with no labels. No Doris code was changed, and no build or runtime test was run. The relevant evidence is: - On master, `SchemaChangeHandler.process` acquires the base-table write lock, installs the light change, and releases the lock before returning. `Alter` calls the MTMV hook afterward. `updateBaseIndexSchema` increments the index schema version, but does not advance the table/partition data visible versions. See [schema publication and unlock](https://github.com/apache/doris/blob/81556a1b5ef5d52412a8380e505238bc744f46a8/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java#L2677-L2704), [schema version update](https://github.com/apache/doris/blob/81556a1b5ef5d52412a8380e505238bc744f46a8/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java#L3734-L3765), and [the later hook](https://github.com/apache/doris/blob/81556a1b5ef5d52412a8380e505238bc744f46a8/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java#L375-L378). - Rewrite checks MV state and partition freshness. OLAP freshness snapshots compare visible versions and table/partition IDs, without the index schema version. Thus a previously fresh MV can still pass these gates during this interval. The planner's table read locks stabilize the schema it observes, but do not couple schema publication to the later MV state transition. See [rewrite eligibility](https://github.com/apache/doris/blob/81556a1b5ef5d52412a8380e505238bc744f46a8/fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRewriteUtil.java#L52-L126), [OLAP snapshots](https://github.com/apache/doris/blob/81556a1b5ef5d52412a8380e505238bc744f46a8/fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java#L3867-L3895), and [planner locking](https://github.com/apache/doris/blob/81556a1b5ef5d52412a8380e505238bc744f46a8/fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java#L1069-L1096). - A cache miss matters: `getOrGenerateCache` can analyze the stored MV SQL against the newly visible schema while the MV still contains pre-change rows. Its cache-generation guard is not a comparison against the base table's schema identity. A warm pre-change plan might instead fail rewrite matching, so eligibility alone does not prove that every query returns incorrect rows. See [cache construction](https://github.com/apache/doris/blob/81556a1b5ef5d52412a8380e505238bc744f46a8/fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java#L633-L693). - Replay publishes the light schema under the table lock without invalidating dependent MVs in that operation. MV invalidation arrives separately through `OP_ALTER_MTMV` / `ALTER_STATUS`. `Env.replayJournal` processes records individually; an already readable observer is not made unreadable for every replay batch. See [light-schema replay](https://github.com/apache/doris/blob/81556a1b5ef5d52412a8380e505238bc744f46a8/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java#L3625-L3649) and [replay/readability handling](https://github.com/apache/doris/blob/81556a1b5ef5d52412a8380e505238bc744f46a8/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java#L3182-L3263). This supports an observer interval as well. One version-specific correction: on the inspected master, `AddColumnOp.needChangeMTMVState()` and `AddColumnsOp.needChangeMTMVState()` both return false. For the issue's ADD example, master therefore skips the hook entirely; that case is not merely a narrower temporary interval. The PR changes ADD to invoke the hook. For hook-triggering operations, master already re-analyzes each dependent MV before its unconditional invalidation. Please separate the master ADD coverage gap from the publication-order race. See [master ADD behavior](https://github.com/apache/doris/blob/81556a1b5ef5d52412a8380e505238bc744f46a8/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/AddColumnOp.java#L120-L123) and [master per-MV analysis/invalidation](https://github.com/apache/doris/blob/81556a1b5ef5d52412a8380e505238bc744f46a8/fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRelationManager.java#L491-L521). To establish the user-visible failure, please provide the exact tested FE commit, table and MV DDL, inserted rows, the persisted MV SQL, the SELECT being rewritten, FE role/topology, and rewrite/cache settings. The sample is schematic: `WHERE` must precede `GROUP BY`, and the pre-change binding of `flag` needs a valid scope. An unqualified reference inside a correlated subquery is a better fixture; [the PR's scope-add regression](https://github.com/apache/doris/blob/45a1894dfe406f611d1cb00b1e00daea38ed2681/regression-test/suites/mtmv_p0/test_drop_unreferenced_column_mtmv.groovy#L178-L206) provides that shape, but does not test the concurrent interval. Suggested next steps: 1. Add a latch-controlled test paused after schema publication/unlock and before the hook. Use a successfully refreshed MV, a cold plan cache (also test supported `mtmv_cache_manage_num=0`), and rows whose result changes with the binding. Capture the chosen plan and compare results with `enable_materialized_view_rewrite=false`. Retain MV state, `SyncWithBaseTables`, schema/data versions, and timestamped FE events. Avoid relying on repeated ALTER/SELECT timing alone. 2. Test a readable observer paused after replaying the schema record and before the MV invalidation record. Record both journal IDs. Include overlapping ALTERs, DDL failure, concurrent refresh completion, restart, and failover in the fix's coverage. 3. Choose a mechanism that also covers replay. A transient barrier needs reference counting across overlapping changes, exception-safe release, and a replay/persistence design. Moving judgement under the leader's write lock alone leaves the observer interval; it also needs consistent multi-table lock ordering and journal waits outside metadata locks. An atomic schema-plus-invalidation replay transition is another option, so schema identity is not the only possible distributed solution. 4. If using schema identity, evaluate the existing base-index ID/schema version as inputs rather than inventing an unrelated counter. Capture the identity of the schema actually used by refresh and validate it before publishing the result. Check it before grace-period or relaxed-consistency shortcuts, handle missing identities conservatively, and cover image/journal compatibility. A whole-schema comparison also rejects otherwise harmless column changes until refresh or explicit revalidation, which should be an intentional tradeoff. Until a fix is validated, disabling transparent MV rewrite for affected sessions (`SET enable_materialized_view_rewrite = false`) is a source-supported way to avoid this rewrite path. This does not establish that a stored MV's rows are current. -- 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]
