[arrow] branch master updated: ARROW-16035: [Java] Handling empty JDBC ResultSet

[lidavidm]

This is an automated email from the ASF dual-hosted git repository.

lidavidm 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 2025673f0c ARROW-16035: [Java] Handling empty JDBC ResultSet
2025673f0c is described below

commit 2025673f0c49b8b8ccd41d5494b29653b57366e4
Author: Todd Farmer <t...@fivefarmers.com>
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 <t...@fivefarmers.com>
    Signed-off-by: David Li <li.david...@gmail.com>
---
 .../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