This is an automated email from the ASF dual-hosted git repository. kszucs pushed a commit to branch release-8.0.0 in repository https://gitbox.apache.org/repos/asf/arrow.git
commit ffd80740ee629cd5240f3da725b3a9e0020b84bf Author: Todd Farmer <[email protected]> AuthorDate: Tue May 3 08:06:27 2022 -0400 ARROW-16035: [Java] Handling empty JDBC ResultSet ArrowVectorIterator.hasNext() delegates to the underlying resultset.isAfterLast() method, but per JDBC specs, this [should return false in the case of empty ResultSets](https://docs.oracle.com/en/java/javase/11/docs/api/java.sql/java/sql/ResultSet.html#isAfterLast()). This causes hasNext() to return true for empty ResultSets, triggering infinite loops. Closes #13049 from toddfarmer/tofarmer/fix-hasnext-for-empty-resultset Authored-by: Todd Farmer <[email protected]> Signed-off-by: David Li <[email protected]> --- .../arrow/adapter/jdbc/ArrowVectorIterator.java | 20 +++++++++++++---- .../arrow/adapter/jdbc/h2/JdbcToArrowTest.java | 25 +++++++++++++++++++--- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/ArrowVectorIterator.java b/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/ArrowVectorIterator.java index 1dfc462b5e..ba0b700415 100644 --- a/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/ArrowVectorIterator.java +++ b/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/ArrowVectorIterator.java @@ -53,6 +53,10 @@ public class ArrowVectorIterator implements Iterator<VectorSchemaRoot>, AutoClos private final int targetBatchSize; + // This is used to track whether the ResultSet has been fully read, and is needed spcifically for cases where there + // is a ResultSet having zero rows (empty): + private boolean readComplete = false; + /** * Construct an instance. */ @@ -107,10 +111,15 @@ public class ArrowVectorIterator implements Iterator<VectorSchemaRoot>, AutoClos compositeConsumer.consume(resultSet); readRowCount++; } + readComplete = true; } else { - while (readRowCount < targetBatchSize && resultSet.next()) { - compositeConsumer.consume(resultSet); - readRowCount++; + while ((readRowCount < targetBatchSize) && !readComplete) { + if (resultSet.next()) { + compositeConsumer.consume(resultSet); + readRowCount++; + } else { + readComplete = true; + } } } @@ -154,8 +163,11 @@ public class ArrowVectorIterator implements Iterator<VectorSchemaRoot>, AutoClos @Override public boolean hasNext() { + if (readComplete) { + return false; + } try { - return !resultSet.isAfterLast(); + return !resultSet.isLast(); } catch (SQLException e) { throw new RuntimeException(e); } diff --git a/java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/h2/JdbcToArrowTest.java b/java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/h2/JdbcToArrowTest.java index ca1c0c00bf..32db254ac3 100644 --- a/java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/h2/JdbcToArrowTest.java +++ b/java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/h2/JdbcToArrowTest.java @@ -40,7 +40,7 @@ import static org.apache.arrow.adapter.jdbc.JdbcToArrowTestHelper.getDoubleValue import static org.apache.arrow.adapter.jdbc.JdbcToArrowTestHelper.getFloatValues; import static org.apache.arrow.adapter.jdbc.JdbcToArrowTestHelper.getIntValues; import static org.apache.arrow.adapter.jdbc.JdbcToArrowTestHelper.getLongValues; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.*; import java.io.IOException; import java.io.InputStream; @@ -251,15 +251,31 @@ public class JdbcToArrowTest extends AbstractJdbcToArrowTest { allocator.close(); } - assertEquals(x, targetRows); + assertEquals(targetRows, x); + } + + @Test + public void testZeroRowResultSet() throws Exception { + BufferAllocator allocator = new RootAllocator(Integer.MAX_VALUE); + int x = 0; + final int targetRows = 0; + ResultSet rs = new FakeResultSet(targetRows); + JdbcToArrowConfig config = new JdbcToArrowConfigBuilder( + allocator, JdbcToArrowUtils.getUtcCalendar(), /* include metadata */ false) + .setReuseVectorSchemaRoot(reuseVectorSchemaRoot).build(); + + ArrowVectorIterator iter = JdbcToArrow.sqlToArrowVectorIterator(rs, config); + assertFalse("Iterator on zero row ResultSet should not haveNext()", iter.hasNext()); } private class FakeResultSet implements ResultSet { public int numRows; + public boolean empty; FakeResultSet(int numRows) { this.numRows = numRows; + this.empty = numRows <= 0; } @Override @@ -629,6 +645,9 @@ public class JdbcToArrowTest extends AbstractJdbcToArrowTest { @Override public boolean isAfterLast() throws SQLException { + if (empty) { + return false; + } return numRows < 0; } @@ -639,7 +658,7 @@ public class JdbcToArrowTest extends AbstractJdbcToArrowTest { @Override public boolean isLast() throws SQLException { - return false; + return numRows <= 0; } @Override
