github-actions[bot] commented on code in PR #66795:
URL: https://github.com/apache/doris/pull/66795#discussion_r3848858688
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/SessionVarGuardRewriter.java:
##########
@@ -71,55 +127,189 @@ private Plan rewritePlanNode(Plan plan) {
return plan;
}
- private static class ReplaceRule implements ExpressionPatternRuleFactory {
+ /**
+ * Applies {@link AddSessionVarGuardRewriter} to the whole expression
tree, so that non-Alias
+ * expressions (e.g. filter predicates, join conjuncts) are guarded as
well as alias children.
+ */
+ private static class AddGuardExpressionRewriteRule implements
ExpressionRewriteRule<ExpressionRewriteContext> {
private final AddSessionVarGuardRewriter addGuardRewriter;
- private ReplaceRule(AddSessionVarGuardRewriter guard) {
- this.addGuardRewriter = guard;
+ private AddGuardExpressionRewriteRule(AddSessionVarGuardRewriter
addGuardRewriter) {
+ this.addGuardRewriter = addGuardRewriter;
}
@Override
- public List<ExpressionPatternMatcher<? extends Expression>>
buildRules() {
- return ImmutableList.of(
- matchesType(Alias.class).thenApply(ctx -> {
- Alias alias = ctx.expr;
- Expression aliasChild =
alias.child().accept(addGuardRewriter, Boolean.FALSE);
- return
alias.withChildren(ImmutableList.of(aliasChild));
- }).toRule(ExpressionRuleType.ADD_SESSION_VAR_GUARD)
- );
+ public Expression rewrite(Expression expr, ExpressionRewriteContext
ctx) {
+ return expr.accept(addGuardRewriter, Boolean.FALSE);
}
}
- /** This ensures that all expressions implementing NeedSessionVarGuard are
- * wrapped in a SessionVarGuardExpr layer.
- * e.g. (a+b)*c -> guard(guard(a+b)*c)
- * */
+ /**
+ * Wraps expressions whose value depends on session variables (or on the
session time zone) in a
+ * {@link SessionVarGuardExpr} when the relevant session variables differ
from the ones persisted on
+ * the object (view / materialized view / generated column) being
processed.
+ */
public static class AddSessionVarGuardRewriter extends
DefaultExpressionRewriter<Boolean> {
private final Map<String, String> sessionVar;
+ // Whether the time-zone family (time-zone sensitive expressions) must
be guarded: the creation
+ // time zone differs from the current one, or the persisted map does
not carry time_zone at all
+ // (pre-time_zone metadata), so the creation zone is unknown and must
be treated as different.
+ private final boolean timeZoneDiffersOrUnknown;
+ // Whether the "other" guard family (NeedSessionVarGuard expressions,
e.g. decimal256 dependent)
+ // must be guarded: some affectQueryResult session variable other than
time_zone differs.
+ private final boolean otherSessionVarsDiffer;
+ // True when the guards are added to a shared materialized-view
rewrite cache (MTMVCache.from).
+ // Cache-mismatch guards must stay structurally distinct from the
guards BindRelation adds when
+ // expanding a persisted object into the query, so that pre-RBO
expression matching never equates a
+ // query-side nested-object guard with the cache guard of an MTMV
materialized in another zone.
+ private final boolean cacheGuard;
+ /**
+ * Creates a guard rewriter that guards both dependency families
unconditionally for the persisted
+ * variables {@code var}. The guard decision must not depend on the
current thread-local session:
+ * the only production caller builds this rewriter inside an {@code
AutoCloseSessionVariable} scope
+ * where the current session already equals {@code var} (so deriving
the decision from the
+ * thread-local session would add no guard at all), while the wrapped
expression is later
+ * translated/executed in a different (load) session.
+ *
+ * @param var the persisted session variables of the object being
processed
+ */
public AddSessionVarGuardRewriter(Map<String, String> var) {
- sessionVar = var;
+ this(var, true, true, false);
+ }
+
+ /**
+ * Creates a guard rewriter for the persisted session variables {@code
var} against the current
+ * query session variables {@code currentVars}.
+ *
+ * @param var the persisted session variables of the object being
processed
+ * @param currentVars the current query session's
affectQueryResultInPlan variables
+ */
+ public AddSessionVarGuardRewriter(Map<String, String> var, Map<String,
String> currentVars) {
+ this(var,
+ var != null && !var.isEmpty()
+ // The creation zone is unknown when the persisted
map has no time_zone key
+ // (pre-change metadata); treat it as different so
time-zone sensitive expressions
+ // are always guarded (compatibility fence).
+ && (!var.containsKey(SessionVariable.TIME_ZONE)
+ ||
!timeZonesEquivalent(var.get(SessionVariable.TIME_ZONE),
+
currentVars.get(SessionVariable.TIME_ZONE))),
+ var != null && !var.isEmpty() &&
differsInNonTimeZoneVars(currentVars, var),
+ false);
+ }
+
+ /**
+ * Creates a guard rewriter that guards the families selected by
{@code guardMask} unconditionally.
+ * Used when building a shared rewrite cache: the guarded cache must
contain the guards regardless
+ * of the session it is generated in, so a cache built in the creation
zone (e.g. by a background
+ * refresh task) is still effective for a query in a different zone.
The produced guards are cache
+ * guards (see {@link #cacheGuard}) so they never equal a query-side
nested-object guard.
+ *
+ * @param var the persisted session variables of the object being
processed
+ * @param guardMask combination of {@link #GUARD_TIME_ZONE} and {@link
#GUARD_OTHER}
+ */
+ public AddSessionVarGuardRewriter(Map<String, String> var, int
guardMask) {
+ this(var, (guardMask & GUARD_TIME_ZONE) != 0, (guardMask &
GUARD_OTHER) != 0, true);
+ }
+
+ private AddSessionVarGuardRewriter(Map<String, String> var, boolean
timeZoneDiffersOrUnknown,
+ boolean otherSessionVarsDiffer, boolean cacheGuard) {
+ this.sessionVar = var;
+ this.timeZoneDiffersOrUnknown = timeZoneDiffersOrUnknown;
+ this.otherSessionVarsDiffer = otherSessionVarsDiffer;
+ this.cacheGuard = cacheGuard;
}
@Override
public Expression visit(Expression expr, Boolean insideGuard) {
Expression rewritten = rewriteChildren(this, expr, Boolean.FALSE);
- if (rewritten instanceof NeedSessionVarGuard &&
!Boolean.TRUE.equals(insideGuard)) {
+ if (needsSessionVarGuard(rewritten) &&
!Boolean.TRUE.equals(insideGuard)) {
if (sessionVar == null) {
return expr;
}
- return new SessionVarGuardExpr(rewritten, sessionVar);
+ return new SessionVarGuardExpr(rewritten, sessionVar,
cacheGuard);
}
return rewritten;
}
@Override
public Expression visitSessionVarGuardExpr(SessionVarGuardExpr expr,
Boolean context) {
Expression child = expr.child().accept(this, Boolean.TRUE);
- if (child != expr.child()) {
- return expr.withChildren(ImmutableList.of(child));
+ Expression guarded = child != expr.child() ?
expr.withChildren(ImmutableList.of(child)) : expr;
+ // A cache-building rewriter must keep a cache-mismatch marker
around an existing non-cache guard:
+ // an MTMV over a view carries the view's query-side guard
(cacheGuard=false) in its definition
+ // plan, so without re-marking it the guarded cache for a
cross-zone query has no cache guard and
+ // the isCacheGuard() rejection in AbstractMaterializedViewRule
never fires - FORCE_IN_RBO would
+ // then substitute a value materialized in the MV's
creation/refresh session. Wrap the existing
+ // guard so the cache plan is structurally distinct from the
query-side plan and the rejection
+ // gates see the marker.
+ if (cacheGuard && !expr.isCacheGuard() && sessionVar != null) {
Review Comment:
[P2] Keep nested cache markers scoped to the selected family
`computeGuardMask` now separates time-zone and other-variable dependencies,
but this branch puts a cache marker around every existing non-cache guard for
any nonzero mask. For example, the relevant plans reduce to:
```text
Query: Project(view_guard[OTHER](decimal_a * decimal_b) AS x) -> Scan
Cache: Project(cache_guard[TZ](view_guard[OTHER](decimal_a * decimal_b)) AS
x) -> Scan
```
A UTC view created with `enable_decimal256=true`, expanded by both a UTC
MTMV and a `+08:00` query using `enable_decimal256=false`, gives both sides the
same persisted decimal semantics; the query differs from the MTMV only in time
zone and the expression has no TIMESTAMPTZ dependency. Nevertheless the outer
marker makes the cache-guard gates reject this safe nested-view rewrite. The
symmetric `GUARD_OTHER` over a time-zone-only view guard has the same problem.
Please retain or re-derive the existing guard's dependency family and add the
outer marker only when it intersects `guardMask`, with a cross-family
safe-rewrite 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]
