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 29, 2022 · d248e3e · d248e3e
1 parent 2e30866
commit d248e3e
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())) {