github-actions[bot] commented on code in PR #64795:
URL: https://github.com/apache/doris/pull/64795#discussion_r3467152429
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/TimestampTzLiteral.java:
##########
@@ -92,72 +129,7 @@ public String toString() {
@Override
public String getStringValue() {
- int scale = getDataType().getScale();
- if (scale <= 0) {
- return super.getStringValue();
- }
-
- if (0 <= year && year <= 9999 && 0 <= month && month <= 99 && 0 <= day
&& day <= 99
- && 0 <= hour && hour <= 99 && 0 <= minute && minute <= 99 && 0
<= second && second <= 99
- && 0 <= microSecond && microSecond <= MAX_MICROSECOND) {
- char[] format = new char[] {
- '0', '0', '0', '0', '-', '0', '0', '-', '0', '0', ' ',
'0', '0', ':', '0', '0', ':', '0', '0',
- '.', '0', '0', '0', '0', '0', '0'};
- int offset = 3;
- long year = this.year;
- while (year > 0) {
- format[offset--] = (char) ('0' + (year % 10));
- year /= 10;
- }
-
- offset = 6;
- long month = this.month;
- while (month > 0) {
- format[offset--] = (char) ('0' + (month % 10));
- month /= 10;
- }
-
- offset = 9;
- long day = this.day;
- while (day > 0) {
- format[offset--] = (char) ('0' + (day % 10));
- day /= 10;
- }
-
- offset = 12;
- long hour = this.hour;
- while (hour > 0) {
- format[offset--] = (char) ('0' + (hour % 10));
- hour /= 10;
- }
-
- offset = 15;
- long minute = this.minute;
- while (minute > 0) {
- format[offset--] = (char) ('0' + (minute % 10));
- minute /= 10;
- }
-
- offset = 18;
- long second = this.second;
- while (second > 0) {
- format[offset--] = (char) ('0' + (second % 10));
- second /= 10;
- }
-
- offset = 19 + scale;
- long microSecond = (int) (this.microSecond / Math.pow(10,
TimeStampTzType.MAX_SCALE - scale));
- while (microSecond > 0) {
- format[offset--] = (char) ('0' + (microSecond % 10));
- microSecond /= 10;
- }
- return String.valueOf(format, 0, 20 + scale);
- }
-
- return String.format("%04d-%02d-%02d %02d:%02d:%02d"
- + (scale > 0 ? ".%0" + scale + "d" : ""),
- year, month, day, hour, minute, second,
Review Comment:
`getStringValue()` now includes the TIMESTAMPTZ `+00:00` suffix, but this
class still inherits `DateTimeLiteral.roundMicroSecond()`. When rounding
overflows, that method reparses `getStringValue()` with
`StandardDateFormat.DATE_TIME_FORMATTER_TO_MICRO_SECOND`, whose pattern is only
`yyyy-MM-dd HH:mm:ss[.ffffff]` and rejects any timezone suffix. For example,
`CAST('2024-01-01 00:00:00.9995 +00:00' AS TIMESTAMPTZ(3))` constructs a
`TimestampTzLiteral`, rounds to `1000000`, and then throws
`DateTimeParseException` on `2024-01-01 00:00:00.000+00:00` instead of
returning `2024-01-01 00:00:01.000+00:00`. Please keep the internal rounding
parse on the suffix-free datetime string or override the overflow handling for
TIMESTAMPTZ.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/TimestampTzLiteral.java:
##########
@@ -64,6 +65,42 @@ 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) {
+ DateTimeV2Literal utcLiteral = (DateTimeV2Literal)
DateTimeExtractAndTransform.convertTz(
+ literal,
+ new StringLiteral(getSessionTimeZone()),
+ new StringLiteral("UTC"));
+ return new TimestampTzLiteral(dateType,
+ utcLiteral.year,
+ utcLiteral.month,
+ utcLiteral.day,
+ utcLiteral.hour,
+ utcLiteral.minute,
+ utcLiteral.second,
+ utcLiteral.microSecond);
+ }
+
+ private static String getSessionTimeZone() {
Review Comment:
This UTC fallback is unsafe for non-session partition callers.
`DynamicPartitionScheduler` computes `prevBorder`/`nextBorder` in
`dynamicPartitionProperty.getTimeZone()` and formats them without an offset,
and TIMESTAMPTZ is allowed there via
`DynamicPartitionUtil.getPartitionFormat()`. When the scheduler calls
`PartitionKey.createPartitionKey`, there is no `ConnectContext`, so
`fromSessionTimeZone()` interprets a table-local boundary like `2024-01-15
00:00:00` as UTC. For a table with `dynamic_partition.time_zone =
'America/New_York'`, that boundary should be stored as `2024-01-15
05:00:00+00:00`, not `2024-01-15 00:00:00+00:00`. Please pass the intended
timezone into this path, or make dynamic partition generation include an
explicit zone/offset before creating the TIMESTAMPTZ partition key.
##########
regression-test/suites/datatype_p0/timestamptz/test_timestamptz_partition_boundary_timezone.groovy:
##########
@@ -0,0 +1,257 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("test_timestamptz_partition_boundary_timezone") {
+ def dbName = "timestamptz_partition_boundary_timezone"
+ def createBoundary = "PARTITION p1 VALUES [('2024-01-15
12:00:00.000000+00:00'), ('2024-01-15 13:00:00.000000+00:00'))"
+ def createBoundary2 = "PARTITION p2 VALUES [('2024-01-15
13:00:00.000000+00:00'), ('2024-01-15 14:00:00.000000+00:00'))"
+
+ sql "DROP DATABASE IF EXISTS ${dbName}"
+ sql "CREATE DATABASE ${dbName}"
+ sql "USE ${dbName}"
+
+ sql "SET enable_nereids_planner = true"
+ sql "SET enable_fallback_to_original_planner = false"
+
+ sql "SET time_zone = 'America/New_York'"
+ sql "DROP TABLE IF EXISTS bug107_create_shift"
+ sql """
+ CREATE TABLE bug107_create_shift (
+ ts TIMESTAMPTZ(6) NOT NULL,
+ seq INT NOT NULL
+ )
+ UNIQUE KEY(ts)
+ PARTITION BY RANGE(ts) (
+ PARTITION p1 VALUES [('2024-01-15 12:00:00 +00:00'), ('2024-01-15
13:00:00 +00:00')),
+ PARTITION p2 VALUES [('2024-01-15 13:00:00 +00:00'), ('2024-01-15
14:00:00 +00:00'))
+ )
+ DISTRIBUTED BY HASH(ts) BUCKETS 1
+ PROPERTIES (
+ "replication_num" = "1",
+ "enable_unique_key_merge_on_write" = "true"
+ )
+ """
+
+ def createShift = sql "SHOW CREATE TABLE bug107_create_shift"
Review Comment:
These deterministic `SHOW CREATE TABLE` boundary checks should be part of
the generated regression output instead of `assertTrue(...contains(...))`. The
local test standard says determined expected results in regression suites
should not use `assert`, and the current `.out` file only shows the
inserted-row query results, not the serialized TIMESTAMPTZ partition clauses
that this suite is trying to protect. Please use a stable `qt_*` query or
equivalent generated output for the DDL boundary text so future serialization
changes are visible in the `.out` diff.
--
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]
