yujun777 commented on code in PR #63639:
URL: https://github.com/apache/doris/pull/63639#discussion_r4322934439
##########
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:
Investigating whether this exposure is introduced by this PR, I found the
root cause predates it and applies to ordinary (non-nested) MVs on current
master:
1. MV dependency tracking only records base tables and views, keyed entirely
by `BaseTableInfo` (`MTMVRelationManager`: `tableMTMVs`, `viewMTMVs`,
`tableMTMVsOneLevelAndFromView`, and the `markIvmBaselineRebuild(...)`
triggers). There is no function dependency map at all.
2. Function DDL (`CREATE OR REPLACE FUNCTION`, drop; `CreateFunctionCommand`
/ `DropFunctionCommand` / `FunctionUtil`) has no hook that invalidates or marks
dependent MVs for rebuild.
Consequence on master, independent of this PR: for a plain MV `SELECT a +
java_udf(b) FROM t`, replacing or dropping `java_udf` does not mark the MV for
rebuild. It keeps serving rows materialized under the old binding while the
definition is re-bound under the new one at plan time — the same wrong-result
class as the scenario above.
So the instability source (a replaceable UDF changing its value without a
refresh) is a general engine gap ("functions are not tracked as an MV
dependency"), the same category as the direct-write limitation already
acknowledged on this thread. This PR's `hasNondeterministic` / `hasAliasUdf`
gates address the in-statement nondeterminism that this PR itself introduces;
the cross-refresh UDF binding drift is downstream of that pre-existing gap and
would be fixed correctly at the dependency-tracking layer, not in the
predicate-elimination gate.
Proposal: track "function change should invalidate/refresh dependent MVs" as
a separate issue rather than fixing it here. Flagging for the author's and
committers' decision — if the preference is a cheap interim mitigation in this
PR, extending the existing gate to also withhold output guarantees when the
definition references a Java/Python UDF is a small, low-risk change.
--
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]
