wgtmac commented on code in PR #35139:
URL: https://github.com/apache/arrow/pull/35139#discussion_r1167977453
##########
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:
To summarize, we have two bugs for now.
- conversion between time zones are incorrect.
- It assumes utc if time zone is not set in the timestamp vector. Same for
varchar vector if it tries to interpret as timestamp values.
For the fix of jdbc implementation of getDate/getTimestamp with a calendar
as input, I’d propose following:
- convert between timezones if timestamp vector has set timezone and user
supplied calendar is not null.
- Throw unsupported operation exception if vector does not have timezone set
and user has provided a timezone. The reason is that the returned timestamp
object cannot carry timezone info. Or return the timestamp as is to pretend it
is in the user supplied timezone?
IIUC, the timezone info in your derby example comes from the
SimpleDateFormat but not the resultset itself.
WDYT? @lidavidm
--
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]