Use Avatica DateTimeUtils for getString in Date/Time (#7)

* Make UTC the default tz instead of using local tz * Use Avatica DateTimeUtils for getString in Date/Time/TimeStamp * Correct usage for unix*ToString * Add unit tests for Date/Time getString * Check wasNull
rafael-telles · Mar 15, 2022 · f5774bf · f5774bf
1 parent e60a4d8
commit f5774bf
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 7 deletions.
diff --git a/...rg/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcDateVectorAccessor.java b/...rg/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcDateVectorAccessor.java
@@ -20,6 +20,8 @@
 import static org.apache.arrow.driver.jdbc.accessor.impl.calendar.ArrowFlightJdbcDateVectorGetter.Getter;
 import static org.apache.arrow.driver.jdbc.accessor.impl.calendar.ArrowFlightJdbcDateVectorGetter.Holder;
 import static org.apache.arrow.driver.jdbc.accessor.impl.calendar.ArrowFlightJdbcDateVectorGetter.createGetter;
+import static org.apache.calcite.avatica.util.DateTimeUtils.MILLIS_PER_DAY;
+import static org.apache.calcite.avatica.util.DateTimeUtils.unixDateToString;
 
 import java.sql.Date;
 import java.sql.Timestamp;
@@ -84,9 +86,7 @@ public Object getObject() {
 
   @Override
   public Date getDate(Calendar calendar) {
-    getter.get(getCurrentRow(), holder);
-    this.wasNull = holder.isSet == 0;
-    this.wasNullConsumer.setWasNull(this.wasNull);
+    fillHolder();
     if (this.wasNull) {
       return null;
     }
@@ -97,6 +97,12 @@ public Date getDate(Calendar calendar) {
     return new Date(DateTimeUtils.applyCalendarOffset(milliseconds, calendar));
   }
 
+  private void fillHolder() {
+    getter.get(getCurrentRow(), holder);
+    this.wasNull = holder.isSet == 0;
+    this.wasNullConsumer.setWasNull(this.wasNull);
+  }
+
   @Override
   public Timestamp getTimestamp(Calendar calendar) {
     Date date = getDate(calendar);
@@ -106,6 +112,16 @@ public Timestamp getTimestamp(Calendar calendar) {
     return new Timestamp(date.getTime());
   }
 
+  @Override
+  public String getString() {
+    fillHolder();
+    if (wasNull) {
+      return null;
+    }
+    long milliseconds = timeUnit.toMillis(holder.value);
+    return unixDateToString((int) (milliseconds / MILLIS_PER_DAY));
+  }
+
   protected static TimeUnit getTimeUnitForVector(ValueVector vector) {
     if (vector instanceof DateDayVector) {
       return TimeUnit.DAYS;

diff --git a/...ache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeStampVectorAccessor.java b/...ache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeStampVectorAccessor.java
@@ -177,7 +177,7 @@ protected static TimeZone getTimeZoneForVector(TimeStampVector vector) {
 
     String timezoneName = arrowType.getTimezone();
     if (timezoneName == null) {
-      return TimeZone.getDefault();
+      return TimeZone.getTimeZone("UTC");
     }
 
     return TimeZone.getTimeZone(timezoneName);

diff --git a/...rg/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeVectorAccessor.java b/...rg/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeVectorAccessor.java
@@ -20,6 +20,8 @@
 import static org.apache.arrow.driver.jdbc.accessor.impl.calendar.ArrowFlightJdbcTimeVectorGetter.Getter;
 import static org.apache.arrow.driver.jdbc.accessor.impl.calendar.ArrowFlightJdbcTimeVectorGetter.Holder;
 import static org.apache.arrow.driver.jdbc.accessor.impl.calendar.ArrowFlightJdbcTimeVectorGetter.createGetter;
+import static org.apache.calcite.avatica.util.DateTimeUtils.MILLIS_PER_DAY;
+import static org.apache.calcite.avatica.util.DateTimeUtils.unixTimeToString;
 
 import java.sql.Time;
 import java.sql.Timestamp;
@@ -116,9 +118,7 @@ public Object getObject() {
 
   @Override
   public Time getTime(Calendar calendar) {
-    getter.get(getCurrentRow(), holder);
-    this.wasNull = holder.isSet == 0;
-    this.wasNullConsumer.setWasNull(this.wasNull);
+    fillHolder();
     if (this.wasNull) {
       return null;
     }
@@ -129,6 +129,12 @@ public Time getTime(Calendar calendar) {
     return new Time(DateTimeUtils.applyCalendarOffset(milliseconds, calendar));
   }
 
+  private void fillHolder() {
+    getter.get(getCurrentRow(), holder);
+    this.wasNull = holder.isSet == 0;
+    this.wasNullConsumer.setWasNull(this.wasNull);
+  }
+
   @Override
   public Timestamp getTimestamp(Calendar calendar) {
     Time time = getTime(calendar);
@@ -138,6 +144,16 @@ public Timestamp getTimestamp(Calendar calendar) {
     return new Timestamp(time.getTime());
   }
 
+  @Override
+  public String getString() {
+    fillHolder();
+    if (wasNull) {
+      return null;
+    }
+    long milliseconds = timeUnit.toMillis(holder.value);
+    return unixTimeToString((int) (milliseconds % MILLIS_PER_DAY));
+  }
+
   protected static TimeUnit getTimeUnitForVector(ValueVector vector) {
     if (vector instanceof TimeNanoVector) {
       return TimeUnit.NANOSECONDS;

diff --git a/...pache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcDateVectorAccessorTest.java b/...pache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcDateVectorAccessorTest.java
@@ -20,6 +20,7 @@
 import static org.apache.arrow.driver.jdbc.accessor.impl.calendar.ArrowFlightJdbcDateVectorAccessor.getTimeUnitForVector;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
 
 import java.sql.Date;
 import java.sql.Timestamp;
@@ -205,6 +206,30 @@ public void testShouldGetStringBeConsistentWithVarCharAccessorWithCalendar() thr
     assertGetStringIsConsistentWithVarCharAccessor(calendar);
   }
 
+  @Test
+  public void testValidateGetStringTimeZoneConsistency() throws Exception {
+    accessorIterator.iterate(vector, (accessor, currentRow) -> {
+      final TimeZone defaultTz = TimeZone.getDefault();
+      try {
+        final String string = accessor.getString(); // Should always be UTC as no calendar is provided
+
+        // Validate with UTC
+        Date date = accessor.getDate(null);
+        TimeZone.setDefault(TimeZone.getTimeZone("UTC"));
+        collector.checkThat(date.toString(), is(string));
+
+        // Validate with different TZ
+        TimeZone.setDefault(TimeZone.getTimeZone(AMERICA_VANCOUVER));
+        collector.checkThat(date.toString(), not(string));
+
+        collector.checkThat(accessor.wasNull(), is(false));
+      } finally {
+        // Set default Tz back
+        TimeZone.setDefault(defaultTz);
+      }
+    });
+  }
+
   private void assertGetStringIsConsistentWithVarCharAccessor(Calendar calendar) throws Exception {
     try (VarCharVector varCharVector = new VarCharVector("",
         rootAllocatorTestRule.getRootAllocator())) {

diff --git a/...pache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeVectorAccessorTest.java b/...pache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcTimeVectorAccessorTest.java
@@ -20,6 +20,7 @@
 import static org.apache.arrow.driver.jdbc.accessor.impl.calendar.ArrowFlightJdbcTimeVectorAccessor.getTimeUnitForVector;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
 
 import java.sql.Time;
 import java.sql.Timestamp;
@@ -214,6 +215,30 @@ public void testShouldGetStringBeConsistentWithVarCharAccessorWithCalendar() thr
     assertGetStringIsConsistentWithVarCharAccessor(calendar);
   }
 
+  @Test
+  public void testValidateGetStringTimeZoneConsistency() throws Exception {
+    accessorIterator.iterate(vector, (accessor, currentRow) -> {
+      final TimeZone defaultTz = TimeZone.getDefault();
+      try {
+        final String string = accessor.getString(); // Should always be UTC as no calendar is provided
+
+        // Validate with UTC
+        Time time = accessor.getTime(null);
+        TimeZone.setDefault(TimeZone.getTimeZone("UTC"));
+        collector.checkThat(time.toString(), is(string));
+
+        // Validate with different TZ
+        TimeZone.setDefault(TimeZone.getTimeZone(AMERICA_VANCOUVER));
+        collector.checkThat(time.toString(), not(string));
+
+        collector.checkThat(accessor.wasNull(), is(false));
+      } finally {
+        // Set default Tz back
+        TimeZone.setDefault(defaultTz);
+      }
+    });
+  }
+
   private void assertGetStringIsConsistentWithVarCharAccessor(Calendar calendar) throws Exception {
     try (VarCharVector varCharVector = new VarCharVector("",
         rootAllocatorTestRule.getRootAllocator())) {