This is an automated email from the ASF dual-hosted git repository. uwe pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push: new c93e2ae ARROW-3923: [Java] JDBC Time Fetches Without Timezone c93e2ae is described below commit c93e2ae14f4d35cf367817a2d2c65024c50dbecc Author: Michael Pigott <mikepig...@users.noreply.github.com> AuthorDate: Tue Feb 5 10:18:41 2019 +0100 ARROW-3923: [Java] JDBC Time Fetches Without Timezone https://issues.apache.org/jira/browse/ARROW-3923 Hello! I was reading through the JDBC source code and I noticed that a java.util.Calendar was required for creating an Arrow Schema and Arrow Vectors from a JDBC ResultSet, when none is required. This change makes the Calendar optional. Unit Tests: The existing SureFire plugin configuration uses a UTC calendar for the database, which is the default Calendar in the existing code. Likewise, no changes to the unit tests are required to provide adequate coverage for the change. Author: Michael Pigott <mikepig...@users.noreply.github.com> Author: Mike Pigott <mpig...@gmail.com> Closes #3066 from mikepigott/jdbc-timestamp-no-calendar and squashes the following commits: 4d95da0a <Mike Pigott> ARROW-3923: Supporting a null Calendar in the config, and reverting the breaking change. cd9a2306 <Mike Pigott> Merge branch 'master' into jdbc-timestamp-no-calendar 509a1cc5 <Michael Pigott> Merge pull request #5 from apache/master 789c8c84 <Michael Pigott> Merge pull request #4 from apache/master e5b19eee <Michael Pigott> Merge pull request #3 from apache/master 3b17c297 <Michael Pigott> Merge pull request #2 from apache/master 881c6c83 <Michael Pigott> Merge pull request #1 from apache/master 089cff4d <Mike Pigott> Format fixes a58a4a5f <Mike Pigott> Fixing calendar usage. e12832a3 <Mike Pigott> Allowing for timestamps without a time zone. --- .../org/apache/arrow/adapter/jdbc/JdbcToArrow.java | 6 ++-- .../arrow/adapter/jdbc/JdbcToArrowConfig.java | 4 +-- .../adapter/jdbc/JdbcToArrowConfigBuilder.java | 11 +++---- .../arrow/adapter/jdbc/JdbcToArrowUtils.java | 38 +++++++++++++++++----- .../arrow/adapter/jdbc/JdbcToArrowConfigTest.java | 15 +++++---- 5 files changed, 46 insertions(+), 28 deletions(-) diff --git a/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrow.java b/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrow.java index fd32036..ddf70df 100644 --- a/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrow.java +++ b/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrow.java @@ -179,13 +179,12 @@ public class JdbcToArrow { * For the given JDBC {@link ResultSet}, fetch the data from Relational DB and convert it to Arrow objects. * * @param resultSet ResultSet to use to fetch the data from underlying database - * @param calendar Calendar instance to use for Date, Time and Timestamp datasets. + * @param calendar Calendar instance to use for Date, Time and Timestamp datasets, or <code>null</code> if none. * @return Arrow Data Objects {@link VectorSchemaRoot} * @throws SQLException on error */ public static VectorSchemaRoot sqlToArrow(ResultSet resultSet, Calendar calendar) throws SQLException, IOException { Preconditions.checkNotNull(resultSet, "JDBC ResultSet object can not be null"); - Preconditions.checkNotNull(calendar, "Calendar object can not be null"); return sqlToArrow(resultSet, new JdbcToArrowConfig(new RootAllocator(Integer.MAX_VALUE), calendar)); } @@ -195,7 +194,7 @@ public class JdbcToArrow { * * @param resultSet ResultSet to use to fetch the data from underlying database * @param allocator Memory allocator to use. - * @param calendar Calendar instance to use for Date, Time and Timestamp datasets. + * @param calendar Calendar instance to use for Date, Time and Timestamp datasets, or <code>null</code> if none. * @return Arrow Data Objects {@link VectorSchemaRoot} * @throws SQLException on error */ @@ -203,7 +202,6 @@ public class JdbcToArrow { throws SQLException, IOException { Preconditions.checkNotNull(resultSet, "JDBC ResultSet object can not be null"); Preconditions.checkNotNull(allocator, "Memory Allocator object can not be null"); - Preconditions.checkNotNull(calendar, "Calendar object can not be null"); return sqlToArrow(resultSet, new JdbcToArrowConfig(allocator, calendar)); } diff --git a/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrowConfig.java b/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrowConfig.java index 59813a8..e9fcffb 100644 --- a/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrowConfig.java +++ b/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrowConfig.java @@ -48,7 +48,6 @@ public final class JdbcToArrowConfig { */ JdbcToArrowConfig(BaseAllocator allocator, Calendar calendar) { Preconditions.checkNotNull(allocator, "Memory allocator cannot be null"); - Preconditions.checkNotNull(calendar, "Calendar object can not be null"); this.allocator = allocator; this.calendar = calendar; @@ -56,7 +55,8 @@ public final class JdbcToArrowConfig { /** * The calendar to use when defining Arrow Timestamp fields - * and retrieving time-based fields from the database. + * and retrieving {@link Date}, {@link Time}, or {@link Timestamp} + * data types from the {@link ResultSet}, or <code>null</code> if not converting. * @return the calendar. */ public Calendar getCalendar() { diff --git a/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrowConfigBuilder.java b/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrowConfigBuilder.java index df97c3a..9ba6963 100644 --- a/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrowConfigBuilder.java +++ b/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrowConfigBuilder.java @@ -32,7 +32,7 @@ public class JdbcToArrowConfigBuilder { /** * Default constructor for the <code>JdbcToArrowConfigBuilder}</code>. - * Use the setter methods for the allocator and calendar; both must be + * Use the setter methods for the allocator and calendar; the allocator must be * set. Otherwise, {@link #build()} will throw a {@link NullPointerException}. */ public JdbcToArrowConfigBuilder() { @@ -41,9 +41,9 @@ public class JdbcToArrowConfigBuilder { } /** - * Constructor for the <code>JdbcToArrowConfigBuilder</code>. Both the - * allocator and calendar are required. A {@link NullPointerException} - * will be thrown if one of the arguments is <code>null</code>. + * Constructor for the <code>JdbcToArrowConfigBuilder</code>. The + * allocator is required, and a {@link NullPointerException} + * will be thrown if it is <code>null</code>. * <p> * The allocator is used to construct Arrow vectors from the JDBC ResultSet. * The calendar is used to determine the time zone of {@link java.sql.Timestamp} @@ -59,7 +59,6 @@ public class JdbcToArrowConfigBuilder { this(); Preconditions.checkNotNull(allocator, "Memory allocator cannot be null"); - Preconditions.checkNotNull(calendar, "Calendar object can not be null"); this.allocator = allocator; this.calendar = calendar; @@ -82,10 +81,8 @@ public class JdbcToArrowConfigBuilder { * Arrow schema, and reading time-based fields from the JDBC <code>ResultSet</code>. * * @param calendar the calendar to set. - * @exception NullPointerExeption if <code>calendar</code> is <code>null</code>. */ public JdbcToArrowConfigBuilder setCalendar(Calendar calendar) { - Preconditions.checkNotNull(calendar, "Calendar object can not be null"); this.calendar = calendar; return this; } diff --git a/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrowUtils.java b/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrowUtils.java index d48cfe2..b6adbbc 100644 --- a/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrowUtils.java +++ b/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrowUtils.java @@ -240,15 +240,15 @@ public class JdbcToArrowUtils { * * @param rs ResultSet to use to fetch the data from underlying database * @param root Arrow {@link VectorSchemaRoot} object to populate - * @param calendar The calendar to use when reading time-based data. + * @param calendar The calendar to use when reading {@link Date}, {@link Time}, or {@link Timestamp} + * data types from the {@link ResultSet}, or <code>null</code> if not converting. * @throws SQLException on error */ public static void jdbcToArrowVectors(ResultSet rs, VectorSchemaRoot root, Calendar calendar) throws SQLException, IOException { Preconditions.checkNotNull(rs, "JDBC ResultSet object can't be null"); - Preconditions.checkNotNull(root, "Vector Schema cannot be null"); - Preconditions.checkNotNull(calendar, "Calendar object can't be null"); + Preconditions.checkNotNull(root, "JDBC ResultSet object can't be null"); jdbcToArrowVectors(rs, root, new JdbcToArrowConfig(new RootAllocator(0), calendar)); } @@ -274,6 +274,8 @@ public class JdbcToArrowUtils { allocateVectors(root, DEFAULT_BUFFER_SIZE); + final Calendar calendar = config.getCalendar(); + int rowCount = 0; while (rs.next()) { for (int i = 1; i <= columnCount; i++) { @@ -324,17 +326,35 @@ public class JdbcToArrowUtils { rs.getString(i), !rs.wasNull(), rowCount); break; case Types.DATE: - updateVector((DateMilliVector) root.getVector(columnName), - rs.getDate(i, config.getCalendar()), !rs.wasNull(), rowCount); + final Date date; + if (calendar != null) { + date = rs.getDate(i, calendar); + } else { + date = rs.getDate(i); + } + + updateVector((DateMilliVector) root.getVector(columnName), date, !rs.wasNull(), rowCount); break; case Types.TIME: - updateVector((TimeMilliVector) root.getVector(columnName), - rs.getTime(i, config.getCalendar()), !rs.wasNull(), rowCount); + final Time time; + if (calendar != null) { + time = rs.getTime(i, calendar); + } else { + time = rs.getTime(i); + } + + updateVector((TimeMilliVector) root.getVector(columnName), time, !rs.wasNull(), rowCount); break; case Types.TIMESTAMP: + final Timestamp ts; + if (calendar != null) { + ts = rs.getTimestamp(i, calendar); + } else { + ts = rs.getTimestamp(i); + } + // TODO: Need to handle precision such as milli, micro, nano - updateVector((TimeStampVector) root.getVector(columnName), - rs.getTimestamp(i, config.getCalendar()), !rs.wasNull(), rowCount); + updateVector((TimeStampVector) root.getVector(columnName), ts, !rs.wasNull(), rowCount); break; case Types.BINARY: case Types.VARBINARY: diff --git a/java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/JdbcToArrowConfigTest.java b/java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/JdbcToArrowConfigTest.java index b4f92fa..1d02c88 100644 --- a/java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/JdbcToArrowConfigTest.java +++ b/java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/JdbcToArrowConfigTest.java @@ -42,14 +42,16 @@ public class JdbcToArrowConfigTest { new JdbcToArrowConfigBuilder(null, null); } - @Test(expected = NullPointerException.class) public void testConfigNullCalendar() { - new JdbcToArrowConfig(allocator, null); + JdbcToArrowConfig config = new JdbcToArrowConfig(allocator, null); + assertNull(config.getCalendar()); } - @Test(expected = NullPointerException.class) + @Test public void testBuilderNullCalendar() { - new JdbcToArrowConfigBuilder(allocator, null); + JdbcToArrowConfigBuilder builder = new JdbcToArrowConfigBuilder(allocator, null); + JdbcToArrowConfig config = builder.build(); + assertNull(config.getCalendar()); } @Test(expected = NullPointerException.class) @@ -68,10 +70,11 @@ public class JdbcToArrowConfigTest { builder.setAllocator(null); } - @Test(expected = NullPointerException.class) + @Test public void testSetNullCalendar() { JdbcToArrowConfigBuilder builder = new JdbcToArrowConfigBuilder(allocator, calendar); - builder.setCalendar(null); + JdbcToArrowConfig config = builder.setCalendar(null).build(); + assertNull(config.getCalendar()); } @Test