lidavidm commented on code in PR #43149:
URL: https://github.com/apache/arrow/pull/43149#discussion_r1687640486
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcDateVectorAccessor.java:
##########
@@ -108,11 +104,36 @@ private void fillHolder() {
@Override
public Timestamp getTimestamp(Calendar calendar) {
- Date date = getDate(calendar);
- if (date == null) {
+ final LocalDateTime localDateTime = getLocalDateTime(calendar);
+ if (localDateTime == null) {
+ return null;
+ }
+
+ return Timestamp.valueOf(localDateTime);
+ }
+
+ private LocalDateTime getLocalDateTime(Calendar calendar) {
+ getter.get(getCurrentRow(), holder);
+ this.wasNull = holder.isSet == 0;
+ this.wasNullConsumer.setWasNull(this.wasNull);
+ if (this.wasNull) {
return null;
}
- return new Timestamp(date.getTime());
+
+ final LocalDateTime localDateTime =
+
DateUtility.getLocalDateTimeFromEpochMilli(this.timeUnit.toMillis(holder.value));
+ final ZoneId defaultTimeZone =
Calendar.getInstance().getTimeZone().toZoneId();
Review Comment:
Sorry, I found a little time to look into this finally.
I still don't see why the system timezone has to come into play.
Arrow date is with reference to the UNIX epoch, i.e. UTC. Java Date is with
reference to GMT. As the Java docs themselves state, they are technically
different but practically the same.
(1) If there is no calendar given, I would expect there to be no datetime
arithmetic done, because they are in the same time zone (effectively). It
happens to work out here because we convert to/from the system timezone, so the
arithmetic here should cancel itself out, but I'm not sure if that will
actually be the case around daylight savings time boundaries.
(2) If there is a calendar given, JDBC is frustratingly vague about the
intention of the parameter. It seems most sources interpret it as, "treat the
date value as a local date (without reference to any timezone) and give me a
timestamp that would display that date (at midnight) in the given timezone". In
that case, again, I don't see why the system timezone should come into play.
--
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]
