Repository: xerial/sqlite-jdbc
Pull request #793 (2022-09-25 13:34:26)

Do not close Statement when checking for more Result Sets (#792)
Possible fix for #792 (PreparedStatement::getMoreResults() closes Prepared Statement itself)


gotson: @hadrabap my attempts to implement multiple result set handling have proven more complicated than expected, mostly because of the intertwining of classes.

Thus i would like to merge your change.

I did a few updates and added more tests, if you could have a look before i merge i would appreciate.

Patch:
diff --git a/src/main/java/org/sqlite/jdbc3/JDBC3Statement.java b/src/main/java/org/sqlite/jdbc3/JDBC3Statement.java
index 897076cb..bc6672ec 100644
--- a/src/main/java/org/sqlite/jdbc3/JDBC3Statement.java
+++ b/src/main/java/org/sqlite/jdbc3/JDBC3Statement.java
@@ -6,6 +6,7 @@ import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.SQLFeatureNotSupportedException;
 import java.sql.SQLWarning;
+import java.sql.Statement;
 import java.util.Arrays;
 import org.sqlite.ExtendedCommand;
 import org.sqlite.ExtendedCommand.SQLExtension;
@@ -18,6 +19,7 @@ public abstract class JDBC3Statement extends CoreStatement {
 
     private int queryTimeout; // in seconds, as per the JDBC spec
     private long updateCount;
+    private boolean exhaustedResults = false;
 
     // PUBLIC INTERFACE /////////////////////////////////////////////
 
@@ -142,6 +144,8 @@ public abstract class JDBC3Statement extends CoreStatement {
     public ResultSet getResultSet() throws SQLException {
         checkOpen();
 
+        if (exhaustedResults) return null;
+
         if (rs.isOpen()) {
             throw new SQLException("ResultSet already requested");
         }
@@ -356,13 +360,29 @@ public abstract class JDBC3Statement extends CoreStatement {
      * @see java.sql.Statement#getMoreResults()
      */
     public boolean getMoreResults() throws SQLException {
-        return getMoreResults(0);
+        return getMoreResults(Statement.CLOSE_CURRENT_RESULT);
     }
 
     /** @see java.sql.Statement#getMoreResults(int) */
-    public boolean getMoreResults(int c) throws SQLException {
+    public boolean getMoreResults(int current) throws SQLException {
         checkOpen();
-        internalClose(); // as we never have another result, clean up pointer
+
+        if (current == Statement.KEEP_CURRENT_RESULT || current == Statement.CLOSE_ALL_RESULTS) {
+            throw new SQLFeatureNotSupportedException(
+                    "Argument not supported: Statement.KEEP_CURRENT_RESULT or Statement.CLOSE_ALL_RESULTS");
+        }
+        if (current != Statement.CLOSE_CURRENT_RESULT) {
+            throw new SQLException("Invalid argument");
+        }
+
+        // we support a single result set, close it
+        rs.close();
+
+        // as we don't have more result, change the update count to -1
+        updateCount = -1;
+        exhaustedResults = true;
+
+        // always return false as we never have more results
         return false;
     }
 


Test patch:
diff --git a/src/test/java/org/sqlite/PrepStmtTest.java b/src/test/java/org/sqlite/PrepStmtTest.java
index 5a1b50b7..ee4e302c 100644
--- a/src/test/java/org/sqlite/PrepStmtTest.java
+++ b/src/test/java/org/sqlite/PrepStmtTest.java
@@ -1,8 +1,6 @@
 package org.sqlite;
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.Assertions.*;
 import static org.assertj.core.data.Offset.offset;
 
 import java.io.ByteArrayInputStream;
@@ -717,4 +715,21 @@ public class PrepStmtTest {
                             });
         }
     }
+
+    @Test
+    public void getMoreResultsDoesNotCloseStatement() throws SQLException {
+        PreparedStatement ps = conn.prepareStatement("select ?");
+        ps.setString(1, "Hello");
+
+        ResultSet rs = ps.executeQuery();
+        assertThat(rs.next()).isTrue();
+        assertThat(rs.getString(1)).isEqualTo("Hello");
+        assertThat(rs.next()).isFalse();
+
+        assertThat(ps.getMoreResults()).isFalse();
+        assertThat(rs.isClosed()).isTrue();
+        assertThat(ps.isClosed()).isFalse();
+
+        assertThatNoException().isThrownBy(ps::clearParameters);
+    }
 }
diff --git a/src/test/java/org/sqlite/StatementTest.java b/src/test/java/org/sqlite/StatementTest.java
index 0fb298d2..e36f3991 100644
--- a/src/test/java/org/sqlite/StatementTest.java
+++ b/src/test/java/org/sqlite/StatementTest.java
@@ -14,6 +14,7 @@ import java.sql.DriverManager;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.sql.SQLFeatureNotSupportedException;
 import java.sql.Statement;
 import java.util.Calendar;
 import org.junit.jupiter.api.AfterEach;
@@ -123,15 +124,21 @@ public class StatementTest {
         assertThat(rs.wasNull()).isTrue();
         assertThat(stat.getMoreResults()).isFalse();
         assertThat(stat.getUpdateCount()).isEqualTo(-1);
+        assertThat(stat.isClosed()).isFalse();
+        assertThat(stat.getResultSet()).isNull();
 
         assertThat(stat.execute("select null;")).isTrue();
         assertThat(stat.getMoreResults()).isFalse();
         assertThat(stat.getUpdateCount()).isEqualTo(-1);
+        assertThat(stat.isClosed()).isFalse();
+        assertThat(stat.getResultSet()).isNull();
 
         assertThat(stat.execute("create table test (c1);")).isFalse();
         assertThat(stat.getUpdateCount()).isEqualTo(0);
         assertThat(stat.getMoreResults()).isFalse();
         assertThat(stat.getUpdateCount()).isEqualTo(-1);
+        assertThat(stat.isClosed()).isFalse();
+        assertThat(stat.getResultSet()).isNull();
     }
 
     @Test
@@ -373,8 +380,28 @@ public class StatementTest {
     public void executeClearRS() throws SQLException {
         assertThat(stat.execute("select null;")).isTrue();
         assertThat(stat.getResultSet()).isNotNull();
+        assertThatExceptionOfType(SQLException.class)
+                .as("requesting the same result set twice should throw an exception")
+                .isThrownBy(() -> stat.getResultSet());
         assertThat(stat.getMoreResults()).isFalse();
-        assertThatExceptionOfType(SQLException.class).isThrownBy(() -> stat.getResultSet());
+        assertThat(stat.isClosed()).isFalse();
+        assertThat(stat.getResultSet()).isNull();
+        assertThat(stat.getUpdateCount()).isEqualTo(-1);
+    }
+
+    @Test
+    public void getMoreResultsArguments() throws SQLException {
+        assertThat(stat.execute("select null;")).isTrue();
+        assertThat(stat.getResultSet()).isNotNull();
+        assertThatExceptionOfType(SQLException.class)
+                .as("getMoreResults only accepts valid arguments")
+                .isThrownBy(() -> stat.getMoreResults(15));
+        assertThatExceptionOfType(SQLFeatureNotSupportedException.class)
+                .as("getMoreResults with CLOSE_ALL_RESULTS is not supported")
+                .isThrownBy(() -> stat.getMoreResults(Statement.CLOSE_ALL_RESULTS));
+        assertThatExceptionOfType(SQLFeatureNotSupportedException.class)
+                .as("getMoreResults with KEEP_CURRENT_RESULT is not supported")
+                .isThrownBy(() -> stat.getMoreResults(Statement.KEEP_CURRENT_RESULT));
     }
 
     @Test