github-actions[bot] commented on code in PR #66795:
URL: https://github.com/apache/doris/pull/66795#discussion_r3840936457
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/SessionVarGuardRewriter.java:
##########
@@ -71,44 +127,107 @@ 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);
Review Comment:
[P1] Preserve the cache mismatch around an existing view guard
For a view created in `+08:00`, an MTMV over it created/refreshed in UTC,
and a query in `-05:00`, the recorded pre-RBO trees are both:
```text
Project(guard[+08,false](date_trunc(ts, 'day')) AS d)
Scan(t)
```
The MTMV's nonzero-mask cache reaches `visitSessionVarGuardExpr`, which
visits the child with `insideGuard=true`; this changed cache-wrapper
construction is therefore never reached and the existing view guard stays
`cacheGuard=false`. Since every new MV gate rejects only `isCacheGuard()`,
`FORCE_IN_RBO` can substitute the UTC-refreshed value, while final guard
merging makes direct execution truncate in `-05:00` (`2024-01-01 02:30Z` lands
on different days). The earlier direct-MTMV case now gets a cache guard, but
this nested-view case gets none. Please retain a cache-mismatch marker around
an existing non-cache guard and add this three-zone regression.
--
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]
