github-actions[bot] commented on code in PR #63639:
URL: https://github.com/apache/doris/pull/63639#discussion_r4131025029
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java:
##########
@@ -875,6 +880,13 @@ private MTMVCache getCache(boolean sessionVarsMatch) {
}
private void setCache(boolean sessionVarsMatch, MTMVCache cache) {
+ // A null cache clears a failed/replayed rebuild. Otherwise output
guarantees are valid
+ // only when the current definition has refreshed every stored
partition. State changes
+ // clear these snapshots; restoring NORMAL after one partition refresh
is not sufficient.
+ if (cache != null && (status.getState() == MTMVState.SCHEMA_CHANGE
Review Comment:
[P1] Suppress output guarantees after direct MTMV writes. For `Query:
Project(id) -> Scan(lower)` and `upper: Project(id) -> Filter(id > 10) ->
Scan(lower)`, this state/snapshot check stays satisfied when
`allow_modify_materialized_view_data=true` inserts id=5 into a refreshed lower
MV defined with `WHERE id > 10`. Refreshing upper then omits id=5, but
`withoutRedundantMvFilters` drops its real filter using the lower's false
guarantee, so the query can rewrite to upper and lose that row. Track direct
writes as a persistent loss of guarantees until a qualifying refresh;
rebuilding the unchanged definition cache repeats the false proof.
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVCache.java:
##########
@@ -142,6 +160,19 @@ public static MTMVCache from(String defSql,
.orElse(rewritePlan);
Pair<Plan, StructInfo> finalPlanStructInfoPair =
constructPlanAndStructInfo(
addGuardRewritePlan, cascadesContext);
+ // Analysis records nondeterministic functions before constant
folding erases their origin.
+ // Replanning current_date(), for example, cannot establish a
guarantee about rows stored
+ // by an earlier refresh. Do not export output guarantees from
such a definition.
+ // Alias UDF definitions can also change between refresh and cache
reconstruction,
+ // even when the expanded function body is deterministic.
+ // A lower MV may have refreshed since this MV's rows were stored.
Derive guarantees
+ // from this definition alone, without importing the lower MV's
current output facts.
+ Set<Expression> outputPredicates =
(cascadesContext.getStatementContext().hasNondeterministic()
+ || cascadesContext.getStatementContext().hasAliasUdf())
Review Comment:
[P1] Preserve refresh-time bindings before exporting output facts. `Upper:
Project(id) -> Filter(id > bound) -> Scan(lower)` becomes a bare lower scan
when this cache claims that filter is redundant. The bound can change without
refreshing lower: an IMMUTABLE Java/Python UDF can be dropped and recreated, or
`@@global.analyze_timeout` can change and be folded again after FE restarts. If
lower stores ids >1 under the old bound and a newly refreshed upper stores only
ids >10, this normalization removes upper's real filter; `SELECT id FROM lower`
can then use upper and lose ids 2..10. The alias-UDF/nondeterministic flags do
not cover these cases. Persist and validate the refresh-time binding, or
withhold the dependent output facts.
--
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]
