924060929 commented on code in PR #64366:
URL: https://github.com/apache/doris/pull/64366#discussion_r3519159102
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnBE.java:
##########
@@ -308,14 +309,15 @@ private static Map<String, Expression>
evalOnBE(Map<String, Map<String, TExpr>>
TNetworkAddress brpcAddress = new TNetworkAddress(be.getHost(),
be.getBrpcPort());
TQueryGlobals queryGlobals = new TQueryGlobals();
-
queryGlobals.setNowString(TimeUtils.getDatetimeFormatWithTimeZone().format(LocalDateTime.now()));
- queryGlobals.setTimestampMs(System.currentTimeMillis());
- queryGlobals.setTimeZone(TimeUtils.DEFAULT_TIME_ZONE);
+ String queryTimeZone = TimeUtils.DEFAULT_TIME_ZONE;
if (context.getSessionVariable().getTimeZone().equals("CST")) {
- queryGlobals.setTimeZone(TimeUtils.DEFAULT_TIME_ZONE);
+ queryTimeZone = TimeUtils.DEFAULT_TIME_ZONE;
} else {
-
queryGlobals.setTimeZone(context.getSessionVariable().getTimeZone());
+ queryTimeZone = context.getSessionVariable().getTimeZone();
Review Comment:
Consistency nit (low priority): the `210e9fd0d57 Reuse timezone alias map`
fix aligned `CoordinatorContext.createQueryGlobals` to resolve the session zone
via `TimeUtils.timeZoneAliasMap.getOrDefault(...)`, but this BE-fold path still
special-cases only `CST` and otherwise sends the raw session zone. So with
`enable_fold_constant_by_be=true` (fuzzed on in p0 CI) and a
`ZoneId.SHORT_IDS`-only alias like `PST`/`JST`, this sends `"PST"` to BE (which
can't resolve it → silently UTC) while `createQueryGlobals` sends
`America/Los_Angeles` — the two BE paths of the same query would then disagree.
It's not reachable in practice today: `FULL_FOLD_REWRITER` runs
`FoldConstantRuleOnFE` first (bottom-up), and every session-tz-dependent
constant function (`now`/`curdate`/`curtime`/`unix_timestamp()`/`utc_*`, plus
`from_unixtime`/`from_second|milli|micro`) already folds to a literal on FE, so
`collectConst`'s `!isLiteral()` guard means nothing tz-dependent reaches
`evalOnBE`. So this is defense-in-depth rather than a live bug — but since the
sibling site was just aligned, mirroring it here closes the asymmetry:
```java
String queryTimeZone = TimeUtils.timeZoneAliasMap.getOrDefault(
context.getSessionVariable().getTimeZone(),
context.getSessionVariable().getTimeZone());
```
(which also subsumes the explicit `CST` branch, since `CST` is in the alias
map).
--
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]
