wgtmac commented on code in PR #35139:
URL: https://github.com/apache/arrow/pull/35139#discussion_r1168088095
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/utils/DateTimeUtils.java:
##########
@@ -47,7 +47,7 @@ public static long applyCalendarOffset(long milliseconds,
Calendar calendar) {
final TimeZone defaultTz = TimeZone.getDefault();
if (tz != defaultTz) {
- milliseconds -= tz.getOffset(milliseconds) -
defaultTz.getOffset(milliseconds);
+ milliseconds += tz.getOffset(milliseconds) -
defaultTz.getOffset(milliseconds);
Review Comment:
Right, at least these behaviors are consistent to each other. It is
unintuitive since timestamp values are always returned in the GMT regardless of
what calendar is supplied. I cannot say this is wrong because it seems to be
aligned with the SQL `TIMESTAMP_WITH_TIMEZONE` type.
There was a
[discussion](https://docs.google.com/document/d/1gNRww9mZJcHvUDCXklzjFEQGpefsuR_akCDfWsdE35Q)
in the Hadoop community to work on the consistency of timestamp semantics
across different SQL-on-Hadoop engines. It proposed a
`TIMESTAMP_WITH_LOCAL_TIMEZONE` type in addition to the SQL
`TIMESTAMP_WITHOUT_TIMEZONE` and `TIMESTAMP_WITH_TIMEZONE` types:
- SQL `TIMESTAMP_WITHOUT_TIMEZONE`: Always display the same wall clock time
regardless of timezone.
- SQL `TIMESTAMP_WITH_TIMEZONE`: The timestamp value is stored in a specific
timezone (can be non-UTC) and can be converted to any timezone for display.
- New `TIMESTAMP_WITH_LOCAL_TIMEZONE`: The timestamp value is stored in the
UTC and always display it in the local timezone.
Most of the SQL engines in the aforementioned doc now support
`TIMESTAMP_WITH_LOCAL_TIMEZONE` type and so do the open file formats like the
Apache Parquet and Apache Orc. I made this PR to align the behavior with
`TIMESTAMP_WITH_LOCAL_TIMEZONE` type. Nevertheless I am no an expert on JDBC so
I am afraid that I may not know what is the expected behavior from the end
users.
Please feel free to close this PR if you think the current behavior is not
an issue, or involve more people to the discussion if a fix is required. Thanks
for the discussion in detail!
--
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]
