Merge pull request apache#40 from rafael-telles/fix-day-and-year-inte…

…rval-format Change the behavior to return the correct format for day and year intervals.
Bit-Quill · May 5, 2022 · 7f1eeec · 7f1eeec
2 parents 5f07a0e + 7f09eb0
commit 7f1eeec
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 38 deletions.
diff --git a/...pache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java b/...pache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java
@@ -19,34 +19,27 @@
 
 import static org.apache.arrow.driver.jdbc.utils.IntervalStringUtils.formatIntervalDay;
 import static org.apache.arrow.driver.jdbc.utils.IntervalStringUtils.formatIntervalYear;
-import static org.joda.time.Period.parse;
+import static org.apache.arrow.vector.util.DateUtility.yearsToMonths;
 
 import java.sql.SQLException;
-import java.time.Duration;
-import java.time.Period;
 import java.util.function.IntSupplier;
 
 import org.apache.arrow.driver.jdbc.accessor.ArrowFlightJdbcAccessor;
 import org.apache.arrow.driver.jdbc.accessor.ArrowFlightJdbcAccessorFactory;
 import org.apache.arrow.vector.BaseFixedWidthVector;
 import org.apache.arrow.vector.IntervalDayVector;
 import org.apache.arrow.vector.IntervalYearVector;
+import org.apache.arrow.vector.holders.NullableIntervalDayHolder;
+import org.apache.arrow.vector.holders.NullableIntervalYearHolder;
+import org.joda.time.Period;
 
 /**
  * Accessor for the Arrow type {@link IntervalDayVector}.
  */
 public class ArrowFlightJdbcIntervalVectorAccessor extends ArrowFlightJdbcAccessor {
 
-  /**
-   * Functional interface used to unify Interval*Vector#getAsStringBuilder implementations.
-   */
-  @FunctionalInterface
-  interface StringBuilderGetter {
-    StringBuilder get(int index);
-  }
-
   private final BaseFixedWidthVector vector;
-  private final StringBuilderGetter stringBuilderGetter;
+  private final StringGetter stringGetter;
   private final Class<?> objectClass;
 
   /**
@@ -61,8 +54,18 @@ public ArrowFlightJdbcIntervalVectorAccessor(IntervalDayVector vector,
                                                ArrowFlightJdbcAccessorFactory.WasNullConsumer setCursorWasNull) {
     super(currentRowSupplier, setCursorWasNull);
     this.vector = vector;
-    this.stringBuilderGetter = vector::getAsStringBuilder;
-    this.objectClass = Duration.class;
+    stringGetter = (index) -> {
+      final NullableIntervalDayHolder holder = new NullableIntervalDayHolder();
+      vector.get(index, holder);
+      if (holder.isSet == 0) {
+        return null;
+      } else {
+        final int days = holder.days;
+        final int millis = holder.milliseconds;
+        return formatIntervalDay(new Period().plusDays(days).plusMillis(millis));
+      }
+    };
+    objectClass = java.time.Duration.class;
   }
 
   /**
@@ -77,39 +80,47 @@ public ArrowFlightJdbcIntervalVectorAccessor(IntervalYearVector vector,
                                                ArrowFlightJdbcAccessorFactory.WasNullConsumer setCursorWasNull) {
     super(currentRowSupplier, setCursorWasNull);
     this.vector = vector;
-    this.stringBuilderGetter = vector::getAsStringBuilder;
-    this.objectClass = Period.class;
+    stringGetter = (index) -> {
+      final NullableIntervalYearHolder holder = new NullableIntervalYearHolder();
+      vector.get(index, holder);
+      if (holder.isSet == 0) {
+        return null;
+      } else {
+        final int interval = holder.value;
+        final int years = (interval / yearsToMonths);
+        final int months = (interval % yearsToMonths);
+        return formatIntervalYear(new Period().plusYears(years).plusMonths(months));
+      }
+    };
+    objectClass = java.time.Period.class;
   }
 
   @Override
-  public Object getObject() {
-    Object object = this.vector.getObject(getCurrentRow());
-    this.wasNull = object == null;
-    this.wasNullConsumer.setWasNull(this.wasNull);
-
-    return object;
+  public Class<?> getObjectClass() {
+    return objectClass;
   }
 
   @Override
-  public Class<?> getObjectClass() {
-    return this.objectClass;
+  public String getString() throws SQLException {
+    String result = stringGetter.get(getCurrentRow());
+    wasNull = result == null;
+    wasNullConsumer.setWasNull(wasNull);
+    return result;
   }
 
   @Override
-  public String getString() throws SQLException {
-    Object object = getObject();
+  public Object getObject() {
+    Object object = vector.getObject(getCurrentRow());
+    wasNull = object == null;
+    wasNullConsumer.setWasNull(wasNull);
+    return object;
+  }
 
-    this.wasNull = object == null;
-    this.wasNullConsumer.setWasNull(this.wasNull);
-    if (object == null) {
-      return null;
-    }
-    if (vector instanceof IntervalDayVector) {
-      return formatIntervalDay(parse(object.toString()));
-    } else if (vector instanceof IntervalYearVector) {
-      return formatIntervalYear(parse(object.toString()));
-    } else {
-      throw new SQLException("Invalid Interval vector instance");
-    }
+  /**
+   * Functional interface used to unify Interval*Vector#getAsStringBuilder implementations.
+   */
+  @FunctionalInterface
+  interface StringGetter {
+    String get(int index);
   }
 }
diff --git a/...e/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessorTest.java b/...e/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessorTest.java
@@ -167,6 +167,8 @@ public void testShouldGetIntervalYear( ) {
 
   @Test
   public void testShouldGetIntervalDay( ) {
+    Assert.assertEquals("-001 00:00:00.000", formatIntervalDay(parse("PT-24H")));
+    Assert.assertEquals("+001 00:00:00.000", formatIntervalDay(parse("PT+24H")));
     Assert.assertEquals("-000 01:00:00.000", formatIntervalDay(parse("PT-1H")));
     Assert.assertEquals("-000 01:00:00.001", formatIntervalDay(parse("PT-1H-0M-00.001S")));
     Assert.assertEquals("-000 01:01:01.000", formatIntervalDay(parse("PT-1H-1M-1S")));
@@ -185,6 +187,14 @@ public void testShouldGetIntervalDay( ) {
     Assert.assertEquals("+000 22:22:22.222", formatIntervalDay(parse("PT+22H22M22.222S")));
   }
 
+  @Test
+  public void testIntervalDayWithJodaPeriodObject() {
+    Assert.assertEquals("+1567 00:00:00.000",
+        formatIntervalDay(new org.joda.time.Period().plusDays(1567)));
+    Assert.assertEquals("-1567 00:00:00.000",
+        formatIntervalDay(new org.joda.time.Period().minusDays(1567)));
+  }
+
   @Test
   public void testShouldGetStringReturnCorrectString() throws Exception {
     accessorIterator.assertAccessorGetter(vector, ArrowFlightJdbcIntervalVectorAccessor::getString,