Fixed closing of ResultSet on prepareStatement and createStatement (#88)

ydb-platform · Nov 13, 2024 · 6c51120 · 6c51120
2 parents 0f626bb + bb95cab
commit 6c51120
Show file tree

Hide file tree

Showing 5 changed files with 114 additions and 12 deletions.
diff --git a/jdbc/src/main/java/tech/ydb/jdbc/context/StaticQueryResult.java b/jdbc/src/main/java/tech/ydb/jdbc/context/StaticQueryResult.java
@@ -89,8 +89,12 @@ public StaticQueryResult(YdbStatement statement, String ast, String plan) {
     }
 
     @Override
-    public void close() {
-        // nothing
+    public void close() throws SQLException {
+        for (ExpressionResult res: results) {
+            if (res.resultSet != null) {
+                res.resultSet.close();
+            }
+        }
     }
 
     @Override

diff --git a/jdbc/src/main/java/tech/ydb/jdbc/impl/BaseYdbStatement.java b/jdbc/src/main/java/tech/ydb/jdbc/impl/BaseYdbStatement.java
@@ -72,7 +72,6 @@ public YdbConnection getConnection() {
     @Override
     public void close() throws SQLException {
         clearBatch();
-        state.close();
         state = YdbQueryResult.EMPTY;
         isClosed = true;
     }
@@ -144,9 +143,7 @@ public int getUpdateCount() throws SQLException {
     }
 
     protected void cleanState() throws SQLException {
-        state.close();
         state = YdbQueryResult.EMPTY;
-
         clearWarnings();
     }
 

diff --git a/jdbc/src/main/java/tech/ydb/jdbc/impl/YdbConnectionImpl.java b/jdbc/src/main/java/tech/ydb/jdbc/impl/YdbConnectionImpl.java
@@ -210,7 +210,6 @@ public int getHoldability() throws SQLException {
     @Override
     public YdbStatement createStatement(int resultSetType, int resultSetConcurrency,
             int resultSetHoldability) throws SQLException {
-        executor.ensureOpened();
         ctx.getTracer().trace("create statement");
         checkStatementParams(resultSetType, resultSetConcurrency, resultSetHoldability);
         return new YdbStatementImpl(this, resultSetType);
@@ -239,9 +238,8 @@ public YdbPreparedStatement prepareStatement(String sql, int autoGeneratedKeys)
 
     private YdbPreparedStatement prepareStatement(String sql, int resultSetType, YdbPrepareMode mode)
             throws SQLException {
-        executor.ensureOpened();
-        validator.clearWarnings();
 
+        validator.clearWarnings();
         ctx.getTracer().trace("prepare statement");
         YdbQuery query = ctx.findOrParseYdbQuery(sql);
         YdbPreparedQuery params = ctx.findOrPrepareParams(query, mode);

diff --git a/jdbc/src/test/java/tech/ydb/jdbc/impl/YdbLazyResultSetImplTest.java b/jdbc/src/test/java/tech/ydb/jdbc/impl/YdbLazyResultSetImplTest.java
@@ -10,6 +10,7 @@
 import java.net.URL;
 import java.sql.Date;
 import java.sql.JDBCType;
+import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
@@ -57,9 +58,8 @@
 import tech.ydb.table.values.Value;
 import tech.ydb.test.junit5.YdbHelperExtension;
 
-
-
 public class YdbLazyResultSetImplTest {
+
     @RegisterExtension
     private static final YdbHelperExtension ydb = new YdbHelperExtension();
 
@@ -125,7 +125,6 @@ public void close() throws SQLException  {
         Assertions.assertTrue(resultSet.isClosed());
     }
 
-
     @Test
     public void unwrap() throws SQLException {
         Assertions.assertTrue(resultSet.isWrapperFor(YdbResultSet.class));
@@ -265,6 +264,58 @@ public void moveOnEmptyResultSet() throws SQLException {
         }
     }
 
+    @Test
+    public void closeResultSetOnExecuteNext() throws SQLException {
+        ResultSet rs1 = statement.executeQuery(TEST_TABLE.selectSQL());
+        Assertions.assertFalse(rs1.isClosed());
+        ResultSet rs2 = statement.executeQuery(TEST_TABLE.selectSQL());
+        Assertions.assertTrue(rs1.isClosed());
+        Assertions.assertFalse(rs2.isClosed());
+
+        rs2.close();
+    }
+
+    @Test
+    public void closeResultSetOnCreateStatement() throws SQLException {
+        ResultSet rs1 = statement.executeQuery(TEST_TABLE.selectSQL());
+        Assertions.assertFalse(rs1.isClosed());
+
+        Statement other = jdbc.connection().createStatement();
+
+        Assertions.assertFalse(rs1.isClosed()); // new statement doesn't close current result set
+
+        ResultSet rs2 = other.executeQuery(TEST_TABLE.selectSQL());
+        Assertions.assertTrue(rs1.isClosed());
+        Assertions.assertFalse(rs2.isClosed());
+
+        other.close();
+        Assertions.assertFalse(rs2.isClosed());
+
+        rs2.close();
+    }
+
+    @Test
+    public void closeResultSetOnPrepareStatement() throws SQLException {
+        ResultSet rs1 = statement.executeQuery(TEST_TABLE.selectSQL());
+        Assertions.assertFalse(rs1.isClosed());
+
+        PreparedStatement ps = jdbc.connection().prepareStatement(TEST_TABLE.selectAllByKey("?"));
+
+        Assertions.assertFalse(rs1.isClosed()); // prepare statement doesn't close current result set
+        ps.setInt(1, 1);
+
+        Assertions.assertFalse(rs1.isClosed()); // prepare statement doesn't close current result set
+
+        ResultSet rs2 = ps.executeQuery();
+        Assertions.assertTrue(rs1.isClosed());
+        Assertions.assertFalse(rs2.isClosed());
+
+        ps.close();
+        Assertions.assertFalse(rs2.isClosed());
+
+        rs2.close();
+    }
+
     @Test
     public void next() throws SQLException {
         Assertions.assertEquals(0, resultSet.getRow());

diff --git a/jdbc/src/test/java/tech/ydb/jdbc/impl/YdbResultSetImplTest.java b/jdbc/src/test/java/tech/ydb/jdbc/impl/YdbResultSetImplTest.java
@@ -10,6 +10,7 @@
 import java.net.URL;
 import java.sql.Date;
 import java.sql.JDBCType;
+import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
@@ -125,7 +126,6 @@ public void close() throws SQLException  {
         Assertions.assertTrue(resultSet.isClosed());
     }
 
-
     @Test
     public void unwrap() throws SQLException {
         Assertions.assertTrue(resultSet.isWrapperFor(YdbResultSet.class));
@@ -295,6 +295,58 @@ public void moveOnEmptyResultSet() throws SQLException {
         }
     }
 
+    @Test
+    public void closeResultSetOnExecuteNext() throws SQLException {
+        ResultSet rs1 = statement.executeQuery(TEST_TABLE.selectSQL());
+        Assertions.assertFalse(rs1.isClosed());
+        ResultSet rs2 = statement.executeQuery(TEST_TABLE.selectSQL());
+        Assertions.assertTrue(rs1.isClosed());
+        Assertions.assertFalse(rs2.isClosed());
+
+        rs2.close();
+    }
+
+    @Test
+    public void closeResultSetOnCreateStatement() throws SQLException {
+        ResultSet rs1 = statement.executeQuery(TEST_TABLE.selectSQL());
+        Assertions.assertFalse(rs1.isClosed());
+
+        Statement other = jdbc.connection().createStatement();
+
+        Assertions.assertFalse(rs1.isClosed()); // new statement doesn't close current result set
+
+        ResultSet rs2 = other.executeQuery(TEST_TABLE.selectSQL());
+        Assertions.assertTrue(rs1.isClosed());
+        Assertions.assertFalse(rs2.isClosed());
+
+        other.close();
+        Assertions.assertFalse(rs2.isClosed());
+
+        rs2.close();
+    }
+
+    @Test
+    public void closeResultSetOnPrepareStatement() throws SQLException {
+        ResultSet rs1 = statement.executeQuery(TEST_TABLE.selectSQL());
+        Assertions.assertFalse(rs1.isClosed());
+
+        PreparedStatement ps = jdbc.connection().prepareStatement(TEST_TABLE.selectAllByKey("?"));
+
+        Assertions.assertFalse(rs1.isClosed()); // prepare statement doesn't close current result set
+        ps.setInt(1, 1);
+
+        Assertions.assertFalse(rs1.isClosed()); // prepare statement doesn't close current result set
+
+        ResultSet rs2 = ps.executeQuery();
+        Assertions.assertTrue(rs1.isClosed());
+        Assertions.assertFalse(rs2.isClosed());
+
+        ps.close();
+        Assertions.assertFalse(rs2.isClosed());
+
+        rs2.close();
+    }
+
     @Test
     public void next() throws SQLException {
         Assertions.assertEquals(0, resultSet.getRow());