JDBC-531 Fix cached fetcher doesn't close cursor when exceptions occur

FirebirdSQL · Sep 12, 2018 · 89baee5 · 89baee5
1 parent eb3dcda
commit 89baee5
Show file tree

Hide file tree

Showing 3 changed files with 95 additions and 37 deletions.
diff --git a/src/etc/release_notes.md b/src/etc/release_notes.md
@@ -62,6 +62,9 @@ Changelog
 
 The following has been changed or fixed in Jaybird 2.2.15:
 
+-   Fixed: Exceptions during fetch of cached result sets (holdable over commit, scrollable and 
+    metadata) prevented prepared statement reuse/re-execute with error _"Attempt to reopen an open 
+    cursor"_ ([JDBC-531](http://tracker.firebirdsql.org/browse/JDBC-531))
 -   Fixed: Jaybird cannot parse Firebird version numbers with revisions ([JDBC-534](http://tracker.firebirdsql.org/browse/JDBC-534))
 -   Fixed: Incorrect parsing of Firebird version numbers ([JDBC-535](http://tracker.firebirdsql.org/browse/JDBC-535))
 

diff --git a/src/main/org/firebirdsql/jdbc/FBCachedFetcher.java b/src/main/org/firebirdsql/jdbc/FBCachedFetcher.java
@@ -54,48 +54,50 @@ class FBCachedFetcher implements FBFetcher {
             if (fetchSize == 0)
                 fetchSize = MAX_FETCH_ROWS;
             this.fetchSize = fetchSize;
-
-            // the following if, is only for callable statement				
-            if (!stmt_handle.isAllRowsFetched() && stmt_handle.size() == 0) {
-                do {
-                    if (maxRows != 0 && fetchSize > maxRows - rowsCount)
-                        fetchSize = maxRows - rowsCount;
-                    gdsHelper.fetch(stmt_handle, fetchSize);
-
-                    final int fetchedRowCount = stmt_handle.size();
-                    if (fetchedRowCount > 0){
-                        byte[][][] rows = stmt_handle.getRows();
-                        // Copy of right length when less rows fetched than requested
-                        if (rows.length > fetchedRowCount) {
-                            final byte[][][] tempRows = new byte[fetchedRowCount][][];
-                            System.arraycopy(rows, 0, tempRows, 0, fetchedRowCount);
-                            rows = tempRows;
+            try {
+                // the following if, is only for callable statement
+                if (!stmt_handle.isAllRowsFetched() && stmt_handle.size() == 0) {
+                    do {
+                        if (maxRows != 0 && fetchSize > maxRows - rowsCount)
+                            fetchSize = maxRows - rowsCount;
+                        gdsHelper.fetch(stmt_handle, fetchSize);
+
+                        final int fetchedRowCount = stmt_handle.size();
+                        if (fetchedRowCount > 0) {
+                            byte[][][] rows = stmt_handle.getRows();
+                            // Copy of right length when less rows fetched than requested
+                            if (rows.length > fetchedRowCount) {
+                                final byte[][][] tempRows = new byte[fetchedRowCount][][];
+                                System.arraycopy(rows, 0, tempRows, 0, fetchedRowCount);
+                                rows = tempRows;
+                            }
+                            rowsSets.add(rows);
+                            rowsCount += fetchedRowCount;
+                            stmt_handle.removeRows();
                         }
-                        rowsSets.add(rows);
-                        rowsCount += fetchedRowCount;
-                        stmt_handle.removeRows();
+                    } while (!stmt_handle.isAllRowsFetched() && (maxRows == 0 || rowsCount < maxRows));
+
+                    // now create one list with known capacity
+                    int rowCount = 0;
+                    rowsArray = new Object[rowsCount];
+                    for (int i = 0; i < rowsSets.size(); i++) {
+                        final Object[] oneSet = (Object[]) rowsSets.get(i);
+                        final int toCopy = Math.min(oneSet.length, rowsCount - rowCount);
+                        System.arraycopy(oneSet, 0, rowsArray, rowCount, toCopy);
+                        rowCount += toCopy;
                     }
-                } while (!stmt_handle.isAllRowsFetched() && (maxRows == 0 || rowsCount <maxRows));
-
-                // now create one list with known capacity					 
-                int rowCount = 0;
-                rowsArray = new Object[rowsCount];
-                for (int i = 0; i < rowsSets.size(); i++){
-                    final Object[] oneSet = (Object[]) rowsSets.get(i);
-                    final int toCopy = Math.min(oneSet.length, rowsCount - rowCount);
-                    System.arraycopy(oneSet, 0, rowsArray, rowCount, toCopy);
-                    rowCount += toCopy;
+                    rowsSets.clear();
+                } else {
+                    rowsArray = stmt_handle.getRows();
+                    stmt_handle.removeRows();
                 }
-                rowsSets.clear();
-            } else {
-                rowsArray = stmt_handle.getRows();
-                stmt_handle.removeRows();
-            }
 
-            if (hasBlobs){
-                cacheBlobs(gdsHelper, xsqlvars, isBlob);
+                if (hasBlobs) {
+                    cacheBlobs(gdsHelper, xsqlvars, isBlob);
+                }
+            } finally {
+                gdsHelper.closeStatement(stmt_handle, false);
             }
-            gdsHelper.closeStatement(stmt_handle, false);
         } catch (GDSException ge) {
             throw new FBSQLException(ge);
         }

diff --git a/src/test/org/firebirdsql/jdbc/TestFBPreparedStatement.java b/src/test/org/firebirdsql/jdbc/TestFBPreparedStatement.java
@@ -19,6 +19,7 @@
 package org.firebirdsql.jdbc;
 
 import org.firebirdsql.common.FBJUnit4TestBase;
+import org.firebirdsql.gds.ISCConstants;
 import org.junit.*;
 import org.junit.rules.ExpectedException;
 
@@ -32,6 +33,7 @@
 import static org.firebirdsql.common.DdlHelper.executeDDL;
 import static org.firebirdsql.common.FBTestProperties.*;
 import static org.firebirdsql.common.JdbcResourceHelper.closeQuietly;
+import static org.hamcrest.CoreMatchers.allOf;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.Matchers.greaterThan;
 import static org.junit.Assert.*;
@@ -1121,6 +1123,57 @@ public void testExecuteBatchThrowsExceptionWhenGeneratedKeysSet() throws Excepti
 
     // Other closeOnCompletion behavior considered to be sufficiently tested in TestFBStatement
 
+    /**
+     * Tests if reexecuting a prepared statement after fetch failure works for holdable result set.
+     * <p>
+     * See <a href="http://tracker.firebirdsql.org/browse/JDBC-531">JDBC-531</a>
+     * </p>
+     */
+    @Test
+    public void testReexecuteStatementAfterFailure() throws Exception {
+        executeDDL(con, "recreate table encoding_error ("
+                + " id integer primary key, "
+                + " stringcolumn varchar(10) character set NONE"
+                + ")");
+        PreparedStatement pstmt = con.prepareStatement("insert into encoding_error (id, stringcolumn) values (?, ?)");
+        try {
+            pstmt.setInt(1, 1);
+            pstmt.setBytes(2, new byte[] { (byte) 0xFF, (byte) 0xFF });
+            pstmt.executeUpdate();
+
+            pstmt.setInt(1, 2);
+            pstmt.executeUpdate();
+        } finally {
+            pstmt.close();
+        }
+        con.setAutoCommit(false);
+
+        pstmt = con.prepareStatement(
+                "select cast(stringcolumn as varchar(10) character set UTF8) from encoding_error",
+                ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY, ResultSet.HOLD_CURSORS_OVER_COMMIT);
+        try {
+            try {
+                ResultSet rs = pstmt.executeQuery();
+
+                rs.next();
+            } catch (SQLException e) {
+                // ignore
+            }
+
+            try {
+                ResultSet rs2 = pstmt.executeQuery();
+                rs2.next();
+            } catch (SQLException e) {
+                if (e.getErrorCode() != 335544849) {
+                    e.printStackTrace();
+                    fail("Unexpected exception: " + e);
+                }
+            }
+        } finally {
+            pstmt.close();
+        }
+    }
+
     private void prepareTestData() throws SQLException {
         PreparedStatement pstmt = con.prepareStatement(INSERT_DATA);
         try {