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 HSBC 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.
   
   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]

Reply via email to