github-actions[bot] commented on code in PR #68532:
URL: https://github.com/apache/doris/pull/68532#discussion_r4119708933
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonWriteBinding.java:
##########
@@ -194,13 +196,14 @@ private static String canonicalPartitionValue(
return value;
}
- // Doris writes an LTZ literal as civil time in the session zone.
Paimon 1.3
- // parses the string accepted by withOverwrite in the FE JVM default
zone.
- // Translate the same instant into that zone so the overwrite
predicate and
- // the row written by the JNI writer identify the same typed partition.
- LocalDateTime sessionValue = LocalDateTime.parse(
- value.replace(' ', 'T'),
DateTimeFormatter.ISO_LOCAL_DATE_TIME);
- return sessionValue.atZone(TimeUtils.getDorisZoneId())
+ // The write boundary now carries UTC instants. Do not parse its
display string as
+ // session-local time: that loses the offset and makes a DST fold
ambiguous again.
+ ZonedDateTime instant = literal instanceof TimestampTzLiteral
+ ? ((TimestampTzLiteral)
literal).toJavaDateType().atZone(ZoneId.of("UTC"))
+ : LocalDateTime.parse(value.replace(' ', 'T'),
DateTimeFormatter.ISO_LOCAL_DATE_TIME)
+ .atZone(TimeUtils.getDorisZoneId());
Review Comment:
[P1] The new `TimestampTzLiteral` branch keeps the exact instant only until
this JVM-zone conversion drops its offset. With an FE JVM in
`America/Los_Angeles`, static LTZ partitions for `2023-11-05 08:30Z` and
`09:30Z` both become `2023-11-05 01:30:00` before `PaimonTransaction` calls
`withOverwrite`, so the overwrite cannot identify the intended partition and
may replace the wrong one. Preserve an unambiguous instant through the API, or
reject fold-ambiguous static overwrites; the current test computes the same
lossy string as its expected value.
##########
fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/TrinoJdbcExecutor.java:
##########
@@ -58,6 +60,11 @@ protected void initializeBlock(int columnCount, String[]
replaceStringList, int
@Override
protected Object getColumnValue(int columnIndex, ColumnType type, String[]
replaceStringList) throws SQLException {
switch (type.getType()) {
+ case TIMESTAMPTZ: {
+ // JNI carries instants as UTC components, not the source
zone's wall clock.
Review Comment:
[P1] Trino JDBC 435's `timestamp with time zone` text contains a local wall
time and zone ID but no overlap offset. Its `getObject(...,
ZonedDateTime.class)` reconstructs that text with `LocalDateTime.atZone`, which
chooses the earlier offset. For `America/Los_Angeles` on 2023-11-05, source
instants `08:30Z` and `09:30Z` both reach this new TIMESTAMPTZ branch as
`08:30Z`, silently shifting the later row by an hour; array elements use the
same driver text conversion. Project an unambiguous epoch/UTC value before JDBC
decoding (including arrays), and test both sides of the overlap.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogProperty.java:
##########
@@ -103,37 +105,43 @@ public Map<String, String> getProperties() {
}
/**
- * @return true if varbinary mapping is enabled, false otherwise
+ * @return true because external binary types always retain their binary
semantics
*/
Review Comment:
[P1] During a rolling FE upgrade, a new follower can become read-ready while
the old master still owns catalogs with `false` or missing mapping markers.
These getters now return `true` on that follower, and the JDBC mappings are
unconditional, but `migrateVarbinaryMappingProperties()` journals the markers
only when a new FE becomes master. Queries run locally on a caught-up follower,
so the same external BLOB/TIMESTAMP column is STRING/DATETIME on the old master
and VARBINARY/TIMESTAMPTZ on the new follower throughout that window. Keep the
follower's effective mapping aligned with the persisted policy until a
migration record is replayed, or prevent local reads of affected catalogs
during the window.
--
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]
