github-actions[bot] commented on code in PR #64366:
URL: https://github.com/apache/doris/pull/64366#discussion_r3602957684
##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -4290,6 +4292,11 @@ public void setTimeZone(String timeZone) {
this.timeZone = timeZone;
}
+ public void checkTimeZone(String timeZone) throws DdlException {
+ TimeUtils.checkTimeZoneValidAndStandardize(timeZone);
+ ZoneId.of(timeZone, TimeUtils.timeZoneAliasMap);
Review Comment:
[P1] Store the standardized time-zone value
`checkTimeZoneValidAndStandardize()` accepts Doris's one-digit offset syntax
and returns a canonical value (`+8:00` becomes `+08:00`), but this discards
that result and reparses raw `+8:00`, which `ZoneId.of()` rejects. Normal `SET`
replaces its literal with the canonical result; `SET_VAR` reaches `VariableMgr`
directly and now fails for this valid spelling. Normalize before storage (for
example with a setter/converter used by `VarAttr`) and test that
`SET_VAR(time_zone='+8:00')` stores `+08:00`; merely removing this parse would
leave the raw value to fail later in `refreshStatementTimeZone()`.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnFE.java:
##########
@@ -547,6 +572,132 @@ public Expression visitTryCast(TryCast cast,
ExpressionRewriteContext context) {
}
}
+ @Override
+ public Expression visitNow(Now now, ExpressionRewriteContext context) {
+ now = rewriteChildren(now, context);
+ Optional<Expression> checkedExpr = preProcess(now);
+ if (checkedExpr.isPresent()) {
+ return checkedExpr.get();
+ }
+ Optional<Instant> statementStartTime = getStatementStartTime(context);
+ Optional<ZoneId> statementTimeZone = getStatementTimeZone(context);
+ Optional<Integer> precision = getIntegerPrecision(now);
+ if (!statementStartTime.isPresent() || !statementTimeZone.isPresent()
|| !precision.isPresent()) {
+ return now;
Review Comment:
[P1] Preserve SQL-cache bookkeeping for unfolded time functions
This unchanged return bypasses the SQL-cache nondeterministic bookkeeping in
`FoldConstantForSqlCache`: that rule subclasses this visitor and overrides only
`visitBoundFunction()`, but `Now` now dispatches to `visitNow()` first. A
supported nonliteral form such as `now(kint)` reaches this branch and remains
nondeterministic without setting `cannotProcessExpression`; because the rewrite
root is unchanged, it records no full nondeterministic pair either.
`nondeterministicFunctionChanged()` then returns false for the empty pair list,
so with `enable_sql_cache` a later execution can reuse the first execution's
timestamp. Preserve the SQL-cache post-rewrite bookkeeping for unfolded
specialized time visitors and add a two-execution `now(kint)` cache 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]
