github-actions[bot] commented on code in PR #67064:
URL: https://github.com/apache/doris/pull/67064#discussion_r3841887561
##########
be/src/util/timezone_utils.cpp:
##########
@@ -236,12 +239,12 @@ static bool normalize_offset_string(const std::string&
timezone, bool allow_hour
minute = 0;
}
- if ((!positive && hour > 12) || (positive && hour > 14) || minute >= 60) {
+ if (hour > 14 || minute >= 60 || (hour == 14 && minute != 0)) {
Review Comment:
[P1] Keep the symmetric range scoped to datetime CAST
`normalize_offset_string` is the generic backend lookup, not a CAST-only
helper. It now makes BE `convert_tz(..., '-13:00', ...)` succeed while FE
folding and the existing `test_timev2_fold` regression still reject it. In the
other direction, FE accepts and forwards `UTC+14:30`, but this condition now
rejects it and `RuntimeState::init` ignores the failed lookup, leaving the
wrong timezone object. Please either put the new range in a CAST-specific entry
point or update every FE generic validator and failure path atomically.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/StringLikeLiteral.java:
##########
@@ -359,6 +354,9 @@ protected DateLiteral getDateTimeLiteral(String year,
String month, String date,
if (hourOffset > 14 || hourOffset == 14 && minuteOffset > 0) {
throw new CastException("Time zone offset couldn't be larger
than 14:00");
}
+ if (minuteOffset != 0 && minuteOffset != 30 && minuteOffset != 45)
{
Review Comment:
[P1] Validate the implicit TIMESTAMPTZ coercion path too
This fixes `StringLikeLiteral.uncheckedCastTo`, but comparisons and `IN`
predicates use the parallel `TypeCoercionUtils.characterLiteralTypeCoercion`
path. Its TIMESTAMPTZ branch still calls
`TimestampTzLiteral.fromSessionTimeZone` directly, so strings such as `+08:15`
or `+14:30` become valid typed literals and can match rows, while the same
explicit CAST now errors in strict mode or returns NULL in non-strict mode.
Please reuse the same offset validation for character-literal coercion and add
comparison/`IN` coverage.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/StringLikeLiteral.java:
##########
@@ -141,11 +140,7 @@ protected Expression uncheckedCastTo(DataType targetType)
throws AnalysisExcepti
if (timeStampTzType.getScale() < 0) {
timeStampTzType = TimeStampTzType.forTypeFromString(value);
}
- if (DateTimeChecker.hasTimeZone(value)) {
- return new TimestampTzLiteral(timeStampTzType, value);
- }
- DateTimeV2Literal datetime = (DateTimeV2Literal)
castToDateTime(DateTimeV2Type.MAX, strictCast);
- return TimestampTzLiteral.fromSessionTimeZone(timeStampTzType,
datetime);
+ return castToDateTime(timeStampTzType, strictCast);
Review Comment:
[P1] Preserve CST zone rules on the new fold path
This reroute now reaches `castToDateTime`'s `CST -> +08:00` rewrite. BE
resolves `CST` as `Asia/Shanghai`, which is not historically fixed at +08:00
(for example, 1986-06-15 used +09:00). Thus `cast('1986-06-15 00:00:00 CST' as
timestamptz)` folds to an instant one hour later than the same expression with
`debug_skip_fold_constant=true`. Please resolve CST through the same
alias/rules as BE and add a historical fold/runtime case.
##########
regression-test/suites/datatype_p0/timestamptz/test_timestamptz_cast.groovy:
##########
@@ -35,6 +36,140 @@ suite("test_timestamptz_cast") {
select cast("2020-01-01 23:59:59.999999+08:00" as timestamptz(5));
"""
+ sql " set debug_skip_fold_constant = false; "
+ order_qt_offset_lower_boundary_fold """
+ select
+ cast('2026-01-01 00:00:00 -14:00' as date),
+ cast('2026-01-01 00:00:00 -14:00' as datev2),
+ cast('2026-01-01 00:00:00 -14:00' as datetime),
+ cast('2026-01-01 00:00:00 -14:00' as datetimev2(6)),
+ cast('2026-01-01 00:00:00 -14:00' as timestamptz(6));
+ """
+
+ order_qt_offset_upper_boundary_fold """
+ select
+ cast('2026-01-01 00:00:00 +14:00' as date),
+ cast('2026-01-01 00:00:00 +14:00' as datev2),
+ cast('2026-01-01 00:00:00 +14:00' as datetime),
+ cast('2026-01-01 00:00:00 +14:00' as datetimev2(6)),
+ cast('2026-01-01 00:00:00 +14:00' as timestamptz(6));
+ """
+
+ order_qt_offset_supported_minutes_fold """
+ select
+ cast('2026-01-01 00:00:00 -13:45' as date),
+ cast('2026-01-01 00:00:00 +13:45' as datev2),
+ cast('2026-01-01 00:00:00 -08:30' as datetime),
+ cast('2026-01-01 00:00:00 +08:30' as datetimev2(6)),
+ cast('2026-01-01 00:00:00 -08:45' as timestamptz(6));
+ """
+
+ order_qt_invalid_offset_minutes_fold """
Review Comment:
[P1] Apply the minute policy to UTC/GMT-prefixed offsets
The new cases cover only tokens beginning with a sign. In both BE parser
modes, `UTC+08:17`/`GMT-08:17` enter the timezone-name branch, where generic
`TimezoneUtils` strips the prefix and accepts arbitrary minutes; FE's colon
handling throws and leaves the CAST for that BE path. The prefixed spelling
therefore succeeds while equivalent bare `+08:17` is NULL/error. Please
classify prefixed fixed offsets at the CAST layer and add strict/non-strict
fold/runtime cases here.
--
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]
