github-actions[bot] commented on code in PR #66795:
URL: https://github.com/apache/doris/pull/66795#discussion_r3820101259
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java:
##########
@@ -220,6 +221,22 @@ protected List<Plan> doRewrite(StructInfo queryStructInfo,
CascadesContext casca
MaterializationContext materializationContext) throws
AnalysisException {
List<Plan> rewriteResults = new ArrayList<>();
StructInfo viewStructInfo = materializationContext.getStructInfo();
+ // A guarded rewrite cache is built when the query session differs
from the MV creation session for
+ // some affectQueryResult family (e.g. a different time zone), and
MTMVCache.from wraps the affected
+ // expressions in cache SessionVarGuardExpr. Such a cache must never
be substituted for the query:
+ // the materialized values were computed in the creation session, and
reading them would silently
+ // differ from what the query computes in its own session (e.g.
date_trunc on a timestamptz truncates
+ // to the UTC day in a UTC-created cache while the +08:00 query would
evaluate it in the local zone).
+ // This is the definitive gate for both the CBO path and the pre-RBO
path, where the recorded query
+ // plan may still carry the query-side guard that BindRelation added
while expanding a nested view,
+ // so expression-level matching alone cannot tell the two guards apart.
+ if (containsCacheGuard(viewStructInfo.getOriginalPlan())) {
Review Comment:
[P2] Defer this guard rejection until the rewrite's dependencies are known.
For a UTC MTMV defined as SELECT id, v, date_trunc(ts, 'day') AS d FROM t, the
+08 cache guards only d. A +08 query selecting just id and v has the same row
set and uses only zone-invariant outputs, so projection-subset rewrite is safe,
but this plan-wide scan returns before output mapping/compensation can discard
d. Reject cache guards used by predicates/grouping or mapped outputs, rather
than any unused projection, and cover a mixed-output subset rewrite.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java:
##########
@@ -232,12 +233,14 @@ public boolean addTaskResult(MTMVTask task, MTMVRelation
relation,
if (!isReplay) {
ConnectContext currentContext = ConnectContext.get();
// shouldn't do this while holding mvWriteLock
- // TODO: these two cache compute share something same, can
be simplified in future
- mtmvCacheWithGuard = createRewriteCache(currentContext,
true, true);
- mtmvCacheWithoutGuard = createRewriteCache(currentContext,
true, false);
+ // Only the unguarded cache (guardMask = 0, query session
matches the creation session)
+ // is pre-generated here. The guarded caches (masks 1/2/3)
are generated lazily on first
+ // use by a query whose session differs: their guard
content must be derived from that
+ // query's mask, and is independent of the session this
refresh task happens to run in.
+ mtmvCacheWithoutGuard = createRewriteCache(currentContext,
true,
Review Comment:
[P2] Avoid eagerly building a cache key this MTMV can never select. For any
nonempty pre-change map without time_zone, computeGuardMask always includes
GUARD_TIME_ZONE; missing current plan-variable keys can also make GUARD_OTHER
permanent. This refresh still runs the full ALL_PLAN cache construction for
mask 0 and publishes only that unreachable entry, so the first query must
synchronously perform another full build for mask 1/2/3. Skip mask-0
pre-generation when it is unreachable (or prebuild an actually reachable mask)
and cover the legacy refresh/first-query path.
--
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]
