github-actions[bot] commented on code in PR #66795:
URL: https://github.com/apache/doris/pull/66795#discussion_r4014756766
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CreateMTMVInfo.java:
##########
@@ -172,6 +174,22 @@ public void analyze(ConnectContext ctx) throws Exception {
// the MV is physically created with IVM metadata.
enableIvm = isExplicitIncremental();
analyzeQuery(ctx);
+ // For a TIMESTAMPTZ-partitioned MTMV (date_trunc on a timestamptz
column) the derived partition
+ // boundaries are UTC-aligned (see MTMVPartitionExprDateTrunc).
Persist time_zone=UTC as the effective
+ // creation zone so the background refresh evaluates the partition
key in UTC (matching the UTC
+ // boundaries) and the rewrite guard compares queries against the
same UTC semantics. Without this,
+ // content expressions would be materialized in a zone different
from the persisted one, and queries
+ // in the persisted (non-UTC) zone could rewrite against a cache
built with different semantics.
+ if (usesUtcTimeZonePartition()) {
Review Comment:
[P1] Normalize the effective zone after AUTO analysis too
This block runs only for non-AUTO refresh methods. For `REFRESH AUTO`,
`analyzeAutoRefreshQuery` probes IVM; an aliased `date_trunc(TIMESTAMPTZ)`
partition is classified as EXPR, rejected by `validateIvmPartition`, and then
succeeds through the regular fallback path without ever returning here.
`OlapTableFactory` consequently persists the non-UTC DDL session map, and every
later refresh context restores it even though `MTMVPartitionExprDateTrunc`
generated UTC-aligned physical boundaries. With a `+08:00` definition, a
`2024-01-01 00:30Z` row truncates to `2024-01-01 00:00+08` (`2023-12-31
16:00Z`), so refresh can fail with no matching sink partition or route it into
the preceding UTC partition.
Please apply this normalization once after either analysis arm has finalized
`mvPartitionInfo` (before the single required partition-descriptor generation),
and add a non-UTC `REFRESH AUTO` regression with an aliased `date_trunc`
partition and a UTC-boundary row.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java:
##########
@@ -1062,6 +1102,185 @@ protected boolean checkIfRewritten(Plan plan,
MaterializationContext context) {
&&
context.alreadyRewrite(plan.getGroupExpression().get().getOwnerGroup().getGroupId());
}
+ /**
+ * Whether the materialized view plan carries a cache guard in a row-set /
grouping / ordering
+ * affecting position: a filter or join predicate, a group-by key, an
order-by key or a window
+ * partition/order key. Such a guard means the materialized rows / groups
were derived with the
+ * creation session semantics, so a query in a different session can never
compensate them and the
+ * rewrite must be rejected regardless of which outputs the query selects.
+ */
+ private static boolean containsCacheGuardInRowAffectingExpressions(Plan
plan) {
+ for (Plan node : plan.<Plan>collectToList(p -> true)) {
+ List<Expression> expressions = new ArrayList<>();
+ if (node instanceof LogicalFilter) {
+ expressions.addAll(((LogicalFilter<?>) node).getConjuncts());
+ } else if (node instanceof LogicalJoin) {
+ LogicalJoin<?, ?> join = (LogicalJoin<?, ?>) node;
+ expressions.addAll(join.getHashJoinConjuncts());
+ expressions.addAll(join.getOtherJoinConjuncts());
+ } else if (node instanceof LogicalAggregate) {
+ expressions.addAll(((LogicalAggregate<?>)
node).getGroupByExpressions());
+ } else if (node instanceof LogicalSort) {
+ ((LogicalSort<?>) node).getOrderKeys().stream()
+ .map(OrderKey::getExpr).forEach(expressions::add);
+ } else if (node instanceof LogicalWindow) {
+ expressions.addAll(((LogicalWindow<?>)
node).getWindowExpressions());
+ }
+ if (containsCacheGuard(expressions)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * Whether the rewritten plan consumes a cache-guarded value. The cache
guard lives on the view's
+ * logical output expression (e.g.
Alias(SessionVarGuardExpr(date_trunc(...)))) and its key in the
+ * shuttled output to MV scan mapping. Once the rewrite maps a query
output onto such an expression
+ * the guard itself disappears from the plan in one of two ways: either
the query output is mapped
+ * directly to the materialized column (the rewritten plan references the
guarded MV scan slot), or -
+ * because a query-side nested-object guard never equals the cache guard -
the expression is
+ * recomputed from the guarded output's base columns (the rewritten plan
contains the unguarded
+ * recomputation pattern, e.g. date_trunc(ts#3, 'day') for the guarded
output date_trunc(ts#0,'day')
+ * whose base column ts#0 materializes ts#3). Both consume a value that
the cache declares
+ * creation-session dependent and must be rejected. Guards on view outputs
the query does not read
+ * never reach the rewritten plan and are harmless, which is what makes a
projection-subset rewrite
+ * (selecting only the zone-invariant columns of a UTC MV) safe.
+ */
+ private static boolean rewrittenPlanReadsGuardedOutput(Plan rewrittenPlan,
+ MaterializationContext materializationContext) {
+ if (containsCacheGuard(rewrittenPlan)) {
+ // a cache guard survived into the rewritten plan, e.g. a
compensated predicate that reads the
+ // creation-session value directly
+ return true;
+ }
+ ExpressionMapping shuttledExprToScanExprMapping =
materializationContext.getShuttledExprToScanExprMapping();
+ if (shuttledExprToScanExprMapping == null) {
+ return false;
+ }
+ Multimap<Expression, Expression> outputToScanMapping =
shuttledExprToScanExprMapping.getExpressionMapping();
+ // base column mapping: the view's shuttled base columns (slots) ->
the MV scan columns that
+ // materialize them, e.g. ts#0 -> ts#3
+ Map<Expression, Expression> baseColumnToScanExpr = new HashMap<>();
+ for (Map.Entry<Expression, Expression> entry :
outputToScanMapping.entries()) {
+ if (entry.getKey() instanceof Slot && entry.getValue() instanceof
Slot) {
+ baseColumnToScanExpr.put(entry.getKey(), entry.getValue());
+ }
+ }
+ // the MV scan columns that directly materialize a cache-guarded view
output, and the unguarded
+ // recomputation patterns (base columns already mapped to MV scan
columns) of those outputs
+ Set<Slot> guardedScanSlots = new HashSet<>();
+ Set<Expression> guardedRecomputePatterns = new HashSet<>();
+ for (Expression outputExpr : outputToScanMapping.keySet()) {
+ Optional<SessionVarGuardExpr> guard =
outputExpr.collectFirst(SessionVarGuardExpr.class::isInstance);
+ if (!guard.isPresent() || !guard.get().isCacheGuard()) {
+ continue;
+ }
+ for (Expression scanExpr : outputToScanMapping.get(outputExpr)) {
+ if (scanExpr instanceof Slot) {
+ guardedScanSlots.add((Slot) scanExpr);
+ }
+ }
+ // the guarded shuttled output expression may nest guards (e.g. a
cast over a guarded child),
+ // strip every guard to get the value-shape, then map the base
columns to MV scan columns
+ guardedRecomputePatterns.add(ExpressionUtils.replace(
Review Comment:
[P2] Do not reject safe recomputation from a raw MV column
For an MV `SELECT ts, date_trunc(ts, 'day') AS d FROM t`, the guarded cache
has two independent mappings:
```text
ts -> mv_scan.ts
cacheGuard(date_trunc(ts, 'day')) -> mv_scan.d
```
A cross-zone query for `date_trunc(ts, 'day')` cannot match the guarded
whole-expression key, so `rewriteExpression` descends and safely produces this
reduced plan:
```text
Project(date_trunc(mv_scan.ts, 'day'))
Scan(mv)
```
That evaluates from the raw TIMESTAMPTZ instant in the current query session
and never reads `mv_scan.d`. Adding exactly this expression to
`guardedRecomputePatterns`, and matching the same value shape again in
`queryOutputReadsGuardedOutput`, conflates safe recomputation with consuming
the creation-zone materialized output and disables a valid rewrite. Please make
the fence follow the actually selected MV scan-slot lineage: direct `mv_scan.d`
use must remain rejected, while recomputation using only independent unguarded
scan slots should be allowed and covered by a cross-zone rewrite-success test.
--
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]
