github-actions[bot] commented on code in PR #63281:
URL: https://github.com/apache/doris/pull/63281#discussion_r3246315102
##########
fe/fe-core/src/test/java/org/apache/doris/catalog/PartitionKeyTest.java:
##########
@@ -287,4 +291,30 @@ public void testMaxValueToSql() throws Exception {
PartitionKey key = PartitionKey.createInfinityPartitionKey(allColumns,
true);
Assert.assertEquals("(MAXVALUE, MAXVALUE, MAXVALUE, MAXVALUE,
MAXVALUE, MAXVALUE, MAXVALUE)", key.toSql());
}
Review Comment:
This method declaration is over-indented and should fail FE checkstyle.
```suggestion
public void testTimestampTzPartitionKeyKeepsExplicitOffset() throws
Exception {
```
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/PartitionKey.java:
##########
@@ -147,14 +142,8 @@ private static Literal getDateTimeLiteral(String value,
Type type) throws Analys
} else if (type.isDatetimeV2()) {
return new DateTimeV2Literal(value);
} else if (type.isTimeStampTz()) {
- DateTimeV2Literal literal = new DateTimeV2Literal(value);
- DateTimeV2Literal dtV2Lit = (DateTimeV2Literal)
(DateTimeExtractAndTransform.convertTz(
- literal,
- new
StringLiteral(ConnectContext.get().getSessionVariable().timeZone),
- new StringLiteral("UTC")));
- return new TimestampTzLiteral((TimeStampTzType)
DataType.fromCatalogType(type),
- dtV2Lit.getYear(), dtV2Lit.getMonth(),
dtV2Lit.getDay(),
- dtV2Lit.getHour(), dtV2Lit.getMinute(),
dtV2Lit.getSecond(), dtV2Lit.getMicroSecond());
+ return
Literal.fromLegacyLiteral(DateLiteralUtils.createDateLiteral(
Review Comment:
This changes the existing TIMESTAMPTZ partition-boundary semantics when the
DDL value has no explicit offset. The old path parsed the value as a DATETIMEV2
in the current session time zone and then converted it to UTC, so `SET
time_zone='America/New_York'; PARTITION ... ('2024-01-15 12:00:00')` stored a
17:00 UTC boundary. `DateLiteralUtils.createDateLiteral(..., TIMESTAMPTZ)` only
applies an offset when the string itself contains one; without an offset it
keeps 12:00 as UTC. That will move existing partition boundaries by the session
offset and can route rows to the wrong partition. Please preserve the
session-time-zone conversion for values without an explicit zone/offset, and
add a regression test for that case.
--
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]
