From 55fc9a17d4ac33157e5edc70f7acf8c5debae59a Mon Sep 17 00:00:00 2001 From: Gabriel Escobar Date: Wed, 4 May 2022 14:13:46 -0300 Subject: [PATCH 1/5] Change the behavior to return the correct format for day and year intervals --- ...ArrowFlightJdbcIntervalVectorAccessor.java | 81 ++++++++++--------- ...wFlightJdbcIntervalVectorAccessorTest.java | 2 + 2 files changed, 46 insertions(+), 37 deletions(-) diff --git a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java index d4ed3a4d0d2cb..eb4843735cc51 100644 --- a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java +++ b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java @@ -19,11 +19,9 @@ 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; @@ -31,22 +29,17 @@ 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,14 @@ 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); + 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 +76,47 @@ public ArrowFlightJdbcIntervalVectorAccessor(IntervalYearVector vector, ArrowFlightJdbcAccessorFactory.WasNullConsumer setCursorWasNull) { super(currentRowSupplier, setCursorWasNull); this.vector = vector; - this.stringBuilderGetter = vector::getAsStringBuilder; - this.objectClass = Period.class; - } - - @Override - public Object getObject() { - Object object = this.vector.getObject(getCurrentRow()); - this.wasNull = object == null; - this.wasNullConsumer.setWasNull(this.wasNull); - - return object; + stringGetter = (index) -> { + final NullableIntervalYearHolder holder = new NullableIntervalYearHolder(); + vector.get(index, holder); + 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 Class getObjectClass() { - return this.objectClass; + return objectClass; } @Override public String getString() throws SQLException { - Object object = getObject(); - - this.wasNull = object == null; - this.wasNullConsumer.setWasNull(this.wasNull); - if (object == null) { + if (vector.isNull(getCurrentRow())) { + wasNull = true; + wasNullConsumer.setWasNull(true); 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"); + return stringGetter.get(getCurrentRow()); + } + + @Override + public Object getObject() { + if (vector.isNull(getCurrentRow())) { + wasNull = true; + wasNullConsumer.setWasNull(true); + return null; } + return vector.getObject(getCurrentRow()); + } + + /** + * Functional interface used to unify Interval*Vector#getAsStringBuilder implementations. + */ + @FunctionalInterface + interface StringGetter { + String get(int index); } } diff --git a/java/flight/flight-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessorTest.java b/java/flight/flight-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessorTest.java index 0123d98db8afc..b000382100c35 100644 --- a/java/flight/flight-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessorTest.java +++ b/java/flight/flight-jdbc-driver/src/test/java/org/apache/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"))); From 133d80973f1f9b70acca46ae92a23d68255813f3 Mon Sep 17 00:00:00 2001 From: Gabriel Escobar Date: Thu, 5 May 2022 13:48:32 -0300 Subject: [PATCH 2/5] Test if holder.isSet indicates the value is null and return null directly --- ...ArrowFlightJdbcIntervalVectorAccessor.java | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java index eb4843735cc51..b41f64f17c630 100644 --- a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java +++ b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java @@ -57,9 +57,13 @@ public ArrowFlightJdbcIntervalVectorAccessor(IntervalDayVector vector, stringGetter = (index) -> { final NullableIntervalDayHolder holder = new NullableIntervalDayHolder(); vector.get(index, holder); - final int days = holder.days; - final int millis = holder.milliseconds; - return formatIntervalDay(new Period().plusDays(days).plusMillis(millis)); + 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; } @@ -79,10 +83,14 @@ public ArrowFlightJdbcIntervalVectorAccessor(IntervalYearVector vector, stringGetter = (index) -> { final NullableIntervalYearHolder holder = new NullableIntervalYearHolder(); vector.get(index, holder); - final int interval = holder.value; - final int years = (interval / yearsToMonths); - final int months = (interval % yearsToMonths); - return formatIntervalYear(new Period().plusYears(years).plusMonths(months)); + 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; } From 6347675aca57fe9c242e363d6e9505d49756e271 Mon Sep 17 00:00:00 2001 From: Gabriel Escobar Date: Thu, 5 May 2022 13:50:08 -0300 Subject: [PATCH 3/5] Change the getString and getObject methods --- ...ArrowFlightJdbcIntervalVectorAccessor.java | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java index b41f64f17c630..a889f3a6eb10c 100644 --- a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java +++ b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java @@ -102,22 +102,18 @@ public Class getObjectClass() { @Override public String getString() throws SQLException { - if (vector.isNull(getCurrentRow())) { - wasNull = true; - wasNullConsumer.setWasNull(true); - return null; - } - return stringGetter.get(getCurrentRow()); + String result = stringGetter.get(getCurrentRow()); + wasNull = result == null; + wasNullConsumer.setWasNull(wasNull); + return result; } @Override public Object getObject() { - if (vector.isNull(getCurrentRow())) { - wasNull = true; - wasNullConsumer.setWasNull(true); - return null; - } - return vector.getObject(getCurrentRow()); + Object object = vector.getObject(getCurrentRow()); + wasNull = object == null; + wasNullConsumer.setWasNull(wasNull); + return object; } /** From bd16af204e1d5671571d5d360f9ca235a146659c Mon Sep 17 00:00:00 2001 From: Gabriel Escobar Date: Thu, 5 May 2022 13:51:10 -0300 Subject: [PATCH 4/5] Include a test passing a Joda object --- .../ArrowFlightJdbcIntervalVectorAccessorTest.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/java/flight/flight-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessorTest.java b/java/flight/flight-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessorTest.java index b000382100c35..ea228692202a7 100644 --- a/java/flight/flight-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessorTest.java +++ b/java/flight/flight-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessorTest.java @@ -187,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, From 7f09eb049d91d8a77182be8e2d80a2e7b92e3804 Mon Sep 17 00:00:00 2001 From: Gabriel Escobar Date: Thu, 5 May 2022 15:14:05 -0300 Subject: [PATCH 5/5] Remove checkstyle problem --- .../impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java index a889f3a6eb10c..283dc9160a9e9 100644 --- a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java +++ b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessor.java @@ -57,7 +57,7 @@ public ArrowFlightJdbcIntervalVectorAccessor(IntervalDayVector vector, stringGetter = (index) -> { final NullableIntervalDayHolder holder = new NullableIntervalDayHolder(); vector.get(index, holder); - if (holder.isSet == 0 ) { + if (holder.isSet == 0) { return null; } else { final int days = holder.days; @@ -83,7 +83,7 @@ public ArrowFlightJdbcIntervalVectorAccessor(IntervalYearVector vector, stringGetter = (index) -> { final NullableIntervalYearHolder holder = new NullableIntervalYearHolder(); vector.get(index, holder); - if (holder.isSet == 0 ) { + if (holder.isSet == 0) { return null; } else { final int interval = holder.value;