Merge pull request apache#29 from rafael-telles/Timestamp_fix

Timestamp fix

Author: rafael-telles

java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeStampVectorAccessor.java

@@ -24,8 +24,6 @@
 import java.sql.Date;
 import java.sql.Time;
 import java.sql.Timestamp;
-import java.time.LocalDateTime;
-import java.time.temporal.ChronoUnit;
 import java.util.Calendar;
 import java.util.TimeZone;
 import java.util.concurrent.TimeUnit;
@@ -35,7 +33,6 @@
 import org.apache.arrow.driver.jdbc.accessor.ArrowFlightJdbcAccessorFactory;
 import org.apache.arrow.vector.TimeStampVector;
 import org.apache.arrow.vector.types.pojo.ArrowType;
-import org.apache.arrow.vector.util.DateUtility;
 
 /**
  * Accessor for the Arrow types extending from {@link TimeStampVector}.
@@ -45,16 +42,8 @@ public class ArrowFlightJdbcTimeStampVectorAccessor extends ArrowFlightJdbcAcces
   private final TimeZone timeZone;
   private final Getter getter;
   private final TimeUnit timeUnit;
-  private final LongToLocalDateTime longToLocalDateTime;
   private final Holder holder;
 
-  /**
-   * Functional interface used to convert a number (in any time resolution) to LocalDateTime.
-   */
-  interface LongToLocalDateTime {
-    LocalDateTime fromLong(long value);
-  }
-
   /**
    * Instantiate a ArrowFlightJdbcTimeStampVectorAccessor for given vector.
    */
@@ -67,7 +56,6 @@ public ArrowFlightJdbcTimeStampVectorAccessor(TimeStampVector vector,
 
     this.timeZone = getTimeZoneForVector(vector);
     this.timeUnit = getTimeUnitForVector(vector);
-    this.longToLocalDateTime = getLongToLocalDateTimeForVector(vector, this.timeZone);
   }
 
   @Override
@@ -80,55 +68,56 @@ public Object getObject() {
     return this.getTimestamp(null);
   }
 
-  private LocalDateTime getLocalDateTime(Calendar calendar) {
+  private Long getLocalDateTimeMillis() {
     getter.get(getCurrentRow(), holder);
     this.wasNull = holder.isSet == 0;
     this.wasNullConsumer.setWasNull(this.wasNull);
     if (this.wasNull) {
       return null;
     }
 
-    long value = holder.value;
-
-    LocalDateTime localDateTime = this.longToLocalDateTime.fromLong(value);
+    final long value = holder.value;
+    final long millis = this.timeUnit.toMillis(value);
 
-    if (calendar != null) {
-      TimeZone timeZone = calendar.getTimeZone();
-      long millis = this.timeUnit.toMillis(value);
-      localDateTime = localDateTime
-          .minus(timeZone.getOffset(millis) - this.timeZone.getOffset(millis), ChronoUnit.MILLIS);
-    }
-    return localDateTime;
+    return millis + this.timeZone.getOffset(millis);
   }
 
   @Override
   public Date getDate(Calendar calendar) {
-    LocalDateTime localDateTime = getLocalDateTime(calendar);
-    if (localDateTime == null) {
+    Long millis = getLocalDateTimeMillis();
+    if (millis == null) {
       return null;
     }
 
-    return new Date(Timestamp.valueOf(localDateTime).getTime());
+    return new Date(applyCalendarOffset(calendar, millis));
   }
 
   @Override
   public Time getTime(Calendar calendar) {
-    LocalDateTime localDateTime = getLocalDateTime(calendar);
-    if (localDateTime == null) {
+    Long millis = getLocalDateTimeMillis();
+    if (millis == null) {
       return null;
     }
 
-    return new Time(Timestamp.valueOf(localDateTime).getTime());
+    return new Time(applyCalendarOffset(calendar, millis));
   }
 
   @Override
   public Timestamp getTimestamp(Calendar calendar) {
-    LocalDateTime localDateTime = getLocalDateTime(calendar);
-    if (localDateTime == null) {
+    Long millis = getLocalDateTimeMillis();
+    if (millis == null) {
       return null;
     }
 
-    return Timestamp.valueOf(localDateTime);
+    return new Timestamp(applyCalendarOffset(calendar, millis));
+  }
+
+  private long applyCalendarOffset(final Calendar calendar, final long millis) {
+    if (calendar == null) {
+      return millis - Calendar.getInstance(TimeZone.getDefault()).getTimeZone().getOffset(millis);
+    }
+
+    return millis - calendar.getTimeZone().getOffset(millis) + this.timeZone.getOffset(millis);
   }
 
   protected static TimeUnit getTimeUnitForVector(TimeStampVector vector) {
@@ -149,28 +138,6 @@ protected static TimeUnit getTimeUnitForVector(TimeStampVector vector) {
     }
   }
 
-  protected static LongToLocalDateTime getLongToLocalDateTimeForVector(TimeStampVector vector,
-                                                                       TimeZone timeZone) {
-    String timeZoneID = timeZone.getID();
-
-    ArrowType.Timestamp arrowType =
-        (ArrowType.Timestamp) vector.getField().getFieldType().getType();
-
-    switch (arrowType.getUnit()) {
-      case NANOSECOND:
-        return nanoseconds -> DateUtility.getLocalDateTimeFromEpochNano(nanoseconds, timeZoneID);
-      case MICROSECOND:
-        return microseconds -> DateUtility.getLocalDateTimeFromEpochMicro(microseconds, timeZoneID);
-      case MILLISECOND:
-        return milliseconds -> DateUtility.getLocalDateTimeFromEpochMilli(milliseconds, timeZoneID);
-      case SECOND:
-        return seconds -> DateUtility.getLocalDateTimeFromEpochMilli(
-            TimeUnit.SECONDS.toMillis(seconds), timeZoneID);
-      default:
-        throw new UnsupportedOperationException("Invalid Arrow time unit");
-    }
-  }
-
   protected static TimeZone getTimeZoneForVector(TimeStampVector vector) {
     ArrowType.Timestamp arrowType =
         (ArrowType.Timestamp) vector.getField().getFieldType().getType();

java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/utils/DateTimeUtils.java

@@ -18,6 +18,7 @@
 package org.apache.arrow.driver.jdbc.utils;
 
 import java.util.Calendar;
+import java.util.TimeZone;
 
 /**
  * Datetime utility functions.
@@ -32,7 +33,7 @@ private DateTimeUtils() {
    */
   public static long applyCalendarOffset(long milliseconds, Calendar calendar) {
     if (calendar == null) {
-      return milliseconds;
+      return milliseconds - Calendar.getInstance(TimeZone.getDefault()).getTimeZone().getOffset(milliseconds);
     }
     return milliseconds - calendar.getTimeZone().getOffset(milliseconds);
   }

java/flight/flight-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeStampVectorAccessorTest.java

@@ -176,13 +176,12 @@ public void testShouldGetTimestampReturnValidTimestampWithCalendar() throws Exce
     TimeZone timeZoneForVector = getTimeZoneForVector(vector);
 
     accessorIterator.iterate(vector, (accessor, currentRow) -> {
-      final Timestamp resultWithoutCalendar = accessor.getTimestamp(null);
       final Timestamp result = accessor.getTimestamp(calendar);
 
-      long offset = timeZone.getOffset(resultWithoutCalendar.getTime()) -
-          timeZoneForVector.getOffset(resultWithoutCalendar.getTime());
+      long offset = timeZone.getOffset(result.getTime()) -
+          timeZoneForVector.getOffset(result.getTime());
 
-      collector.checkThat(resultWithoutCalendar.getTime() - result.getTime(), is(offset));
+      collector.checkThat(result.getTime(), is(getTimestampForVector(currentRow).getTime() - offset));
       collector.checkThat(accessor.wasNull(), is(false));
     });
   }
@@ -209,13 +208,12 @@ public void testShouldGetDateReturnValidDateWithCalendar() throws Exception {
     TimeZone timeZoneForVector = getTimeZoneForVector(vector);
 
     accessorIterator.iterate(vector, (accessor, currentRow) -> {
-      final Date resultWithoutCalendar = accessor.getDate(null);
       final Date result = accessor.getDate(calendar);
 
-      long offset = timeZone.getOffset(resultWithoutCalendar.getTime()) -
-          timeZoneForVector.getOffset(resultWithoutCalendar.getTime());
+      long offset = timeZone.getOffset(result.getTime()) -
+          timeZoneForVector.getOffset(result.getTime());
 
-      collector.checkThat(resultWithoutCalendar.getTime() - result.getTime(), is(offset));
+      collector.checkThat(result.getTime(), is(getTimestampForVector(currentRow).getTime() - offset ));
       collector.checkThat(accessor.wasNull(), is(false));
     });
   }
@@ -242,13 +240,12 @@ public void testShouldGetTimeReturnValidTimeWithCalendar() throws Exception {
     TimeZone timeZoneForVector = getTimeZoneForVector(vector);
 
     accessorIterator.iterate(vector, (accessor, currentRow) -> {
-      final Time resultWithoutCalendar = accessor.getTime(null);
       final Time result = accessor.getTime(calendar);
 
-      long offset = timeZone.getOffset(resultWithoutCalendar.getTime()) -
-          timeZoneForVector.getOffset(resultWithoutCalendar.getTime());
+      long offset = timeZone.getOffset(result.getTime()) -
+          timeZoneForVector.getOffset(result.getTime());
 
-      collector.checkThat(resultWithoutCalendar.getTime() - result.getTime(), is(offset));
+      collector.checkThat(result.getTime(), is(getTimestampForVector(currentRow).getTime() - offset ));
       collector.checkThat(accessor.wasNull(), is(false));
     });
   }