morrySnow commented on code in PR #64795:
URL: https://github.com/apache/doris/pull/64795#discussion_r3489180939
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/TimestampTzLiteral.java:
##########
@@ -64,6 +67,62 @@ public TimestampTzLiteral(TimeStampTzType dateType,
roundMicroSecond(dateType.getScale());
}
+ /**
+ * Build a TIMESTAMPTZ literal from a session-local datetime string.
+ * Strings with an explicit zone/offset keep their own timezone semantics;
+ * strings without one are interpreted in the current session timezone and
converted to UTC.
+ */
+ public static TimestampTzLiteral fromSessionTimeZone(TimeStampTzType
dateType, String s) {
+ if (DateTimeChecker.hasTimeZone(s)) {
+ return new TimestampTzLiteral(dateType, s);
+ }
+
+ return fromSessionTimeZone(dateType, new DateTimeV2Literal(s));
+ }
+
+ /**
+ * fromSessionTimeZone
+ */
+ public static TimestampTzLiteral fromSessionTimeZone(TimeStampTzType
dateType, DateTimeV2Literal literal) {
+ return fromTimeZone(dateType, literal, getSessionTimeZone());
+ }
+
+ /**
+ * Build a TIMESTAMPTZ literal from a datetime string, explicitly
specifying the source timezone.
+ * Strings with an explicit zone/offset keep their own timezone semantics;
+ * strings without one are converted from the given timeZone to UTC.
+ */
+ public static TimestampTzLiteral fromTimeZone(TimeStampTzType dateType,
String s, String timeZone) {
+ if (DateTimeChecker.hasTimeZone(s)) {
+ return new TimestampTzLiteral(dateType, s);
+ }
+ return fromTimeZone(dateType, new DateTimeV2Literal(s), timeZone);
+ }
+
+ /**
+ * Build a TIMESTAMPTZ literal from a DateTimeV2Literal, explicitly
specifying the source timezone.
+ */
+ public static TimestampTzLiteral fromTimeZone(TimeStampTzType dateType,
DateTimeV2Literal literal,
+ String timeZone) {
+ DateTimeV2Literal utcLiteral = (DateTimeV2Literal)
DateTimeExtractAndTransform.convertTz(
+ literal,
+ new StringLiteral(timeZone),
+ new StringLiteral("UTC"));
+ return new TimestampTzLiteral(dateType,
+ utcLiteral.year,
+ utcLiteral.month,
Review Comment:
## `getSessionTimeZone()` null-safe fallback to "UTC"
```java
private static String getSessionTimeZone() {
ConnectContext context = ConnectContext.get();
return context == null ? "UTC" : context.getSessionVariable().timeZone;
}
```
This is a significant improvement over the old code which would NPE when
`ConnectContext.get()` returned null (e.g., in background threads). However,
silently defaulting to `"UTC"` means that if `fromSessionTimeZone()` is
inadvertently called in a context where the session timezone should be
something else, the error goes undetected — the value is just interpreted as
UTC.
Consider:
- Logging a warning at DEBUG/TRACE level when the fallback is used, to aid
debugging
- Or adding a `@Nullable` annotation on the return to make the fallback
explicit at call sites
This is a minor concern; the explicit `fromTimeZone()` overload is available
for callers that know their timezone (as used by DynamicPartitionScheduler).
--
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]
