Finally fixed batching error with prepared handle including repro test

microsoft · May 31, 2017 · 4bf4c0c · 4bf4c0c
1 parent 95f5a27
commit 4bf4c0c
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 16 deletions.
diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java
@@ -502,11 +502,8 @@ final void doExecutePreparedStatement(PrepStmtExecCmd command) throws SQLServerE
         }
 
         // Retry execution if existing handle could not be re-used.
-    	int attempt = 0;
-        while (true) {
-
-        	++attempt;
-        	try {
+        for(int attempt = 1; attempt <= 2; ++attempt) {
+            try {
     			// Re-use handle if available, requires parameter definitions which are not available until here.
     			if (reuseCachedHandle(hasNewTypeDefinitions, 1 < attempt)) {
     				hasNewTypeDefinitions = false;
@@ -544,7 +541,8 @@ private boolean retryBasedOnFailedReuseOfCachedHandle(SQLException e, int attemp
         // Only retry based on these error codes:
         // 586: The prepared statement handle %d is not valid in this context.  Please verify that current database, user default schema, and ANSI_NULLS and QUOTED_IDENTIFIER set options are not changed since the handle is prepared.
         // 8179: Could not find prepared statement with handle %d.
-        return 1 == attempt && (586 == e.getErrorCode() || 8179 == e.getErrorCode());
+        // 99586: Error used for testing.
+        return 1 == attempt && (586 == e.getErrorCode() || 8179 == e.getErrorCode() || 99586 == e.getErrorCode());
     }
 
     /**
@@ -2591,18 +2589,16 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th
             }
 
             // Retry execution if existing handle could not be re-used.
-            int attempt = 0;
-            while (true) {
-
-                ++attempt;
+            for(int attempt = 1; attempt <= 2; ++attempt) {
+
                 try {
 
                     // Re-use handle if available, requires parameter definitions which are not available until here.
                     if (reuseCachedHandle(hasNewTypeDefinitions, 1 < attempt)) {
                         hasNewTypeDefinitions = false;
                     }
 
-                    if (numBatchesExecuted < numBatchesPrepared && 1 == attempt) {
+                    if (numBatchesExecuted < numBatchesPrepared) {
                         // assert null != tdsWriter;
                         tdsWriter.writeByte((byte) nBatchStatementDelimiter);
                     }
@@ -2619,8 +2615,7 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th
                     // But we may also need to reprepare the statement if, for example,
                     // the size of a batch's string parameter values changes such
                     // that repreparation is necessary.
-                    if(1 == attempt) 
-                        ++numBatchesPrepared;
+                    ++numBatchesPrepared;
 
                     if (doPrepExec(tdsWriter, batchParam, hasNewTypeDefinitions, hasExistingTypeDefinitions) || numBatchesPrepared == numBatches) {
                         ensureExecuteResultsReader(batchCommand.startResponse(getIsResponseBufferingAdaptive()));
@@ -2657,6 +2652,8 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th
 
                                 // Retry if invalid handle exception.
                                 if (retryBasedOnFailedReuseOfCachedHandle(e, attempt)) {
+                                    // Reset number of batches prepared.
+                                    numBatchesPrepared = numBatchesExecuted;
                                     retry = true;
                                     break;
                                 }
@@ -2673,8 +2670,7 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th
                             batchCommand.updateCounts[numBatchesExecuted] = (-1 == updateCount) ? Statement.SUCCESS_NO_INFO : updateCount;
                             processBatch();
 
-                            if(1 == attempt) 
-                                numBatchesExecuted++;
+                            numBatchesExecuted++;
                         }
                         if(retry)
                             continue; 
@@ -2685,8 +2681,11 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th
                     }
                 }
                 catch(SQLException e) {
-                    if (retryBasedOnFailedReuseOfCachedHandle(e, attempt))
+                    if (retryBasedOnFailedReuseOfCachedHandle(e, attempt)) {
+                        // Reset number of batches prepared.
+                        numBatchesPrepared = numBatchesExecuted;
                         continue;
+                    }
                     else
                         throw e;
                 }

diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/PreparedStatementTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/PreparedStatementTest.java
@@ -145,6 +145,35 @@ public void testStatementPooling() throws SQLException {
             // Test behvaior with statement pooling.
             con.setStatementPoolingCacheSize(10);
 
+            // Test with missing handle failures (fake).
+            this.executeSQL(con, "CREATE TABLE #update1 (col INT);INSERT #update1 VALUES (1);");
+            this.executeSQL(con, "CREATE PROC #updateProc1 AS UPDATE #update1 SET col += 1; IF EXISTS (SELECT * FROM #update1 WHERE col % 5 = 0) THROW 99586, 'Prepared handle GAH!', 1;");
+            try (SQLServerPreparedStatement pstmt = (SQLServerPreparedStatement) con.prepareStatement("#updateProc1")) {
+                for (int i = 0; i < 100; ++i) {
+                    assertSame(1, pstmt.executeUpdate());
+                }
+            }
+
+            // Test batching with missing handle failures (fake).
+            this.executeSQL(con, "CREATE TABLE #update2 (col INT);INSERT #update2 VALUES (1);");
+            this.executeSQL(con, "CREATE PROC #updateProc2 AS UPDATE #update2 SET col += 1; IF EXISTS (SELECT * FROM #update2 WHERE col % 5 = 0) THROW 99586, 'Prepared handle GAH!', 1;");
+            try (SQLServerPreparedStatement pstmt = (SQLServerPreparedStatement) con.prepareStatement("#updateProc2")) {
+                for (int i = 0; i < 100; ++i)
+                    pstmt.addBatch();
+
+                int[] updateCounts = pstmt.executeBatch();
+
+                // Verify update counts are correct
+                for (int i : updateCounts) {
+                    assertSame(1, i);
+                }
+            }
+        }
+
+        try (SQLServerConnection con = (SQLServerConnection)DriverManager.getConnection(connectionString)) {
+            // Test behvaior with statement pooling.
+            con.setStatementPoolingCacheSize(10);
+
             String lookupUniqueifier = UUID.randomUUID().toString();
             String query = String.format("/*statementpoolingtest_%s*/SELECT * FROM sys.tables;", lookupUniqueifier);