From a6eee22a8622378b4f43fcb844238ec2c8689a49 Mon Sep 17 00:00:00 2001 From: Afsaneh Rafighi Date: Mon, 29 Jan 2018 14:41:39 -0800 Subject: [PATCH 01/12] change back to retryLogic and change the default of prepared statement caching to false --- .../sqlserver/jdbc/SQLServerConnection.java | 34 +- .../jdbc/SQLServerConnectionPoolProxy.java | 2 - .../jdbc/SQLServerPreparedStatement.java | 307 ++++++++++-------- .../sqlserver/jdbc/SQLServerStatement.java | 20 -- .../unit/statement/PreparedStatementTest.java | 124 ++----- 5 files changed, 206 insertions(+), 281 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index c8232d71a..edfa63a20 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -83,8 +83,6 @@ // Note all the public functions in this class also need to be defined in SQLServerConnectionPoolProxy. public class SQLServerConnection implements ISQLServerConnection { - boolean contextIsAlreadyChanged = false; - boolean contextChanged = false; long timerExpire; boolean attemptRefreshTokenLocked = false; @@ -277,10 +275,10 @@ static ParsedSQLCacheItem parseAndCacheSQL(Sha1HashKey key, String sql) throws } /** Size of the prepared statement handle cache */ - private int statementPoolingCacheSize = 10; + private int statementPoolingCacheSize = 0; /** Default size for prepared statement caches */ - static final int DEFAULT_STATEMENT_POOLING_CACHE_SIZE = 10; + static final int DEFAULT_STATEMENT_POOLING_CACHE_SIZE = 0; /** Cache of prepared statement handles */ private ConcurrentLinkedHashMap preparedStatementHandleCache; /** Cache of prepared statement parameter metadata */ @@ -3080,8 +3078,6 @@ final void poolCloseEventNotify() throws SQLServerException { checkClosed(); if (catalog != null) { connectionCommand("use " + Util.escapeSQLId(catalog), "setCatalog"); - contextIsAlreadyChanged = true; - contextChanged = true; sCatalog = catalog; } loggerExternal.exiting(getClassNameLogging(), "setCatalog"); @@ -5699,7 +5695,7 @@ final void unprepareUnreferencedPreparedStatementHandles(boolean force) { */ public int getStatementPoolingCacheSize() { return statementPoolingCacheSize; - } + } /** * Returns the current number of pooled prepared statement handles. @@ -5726,6 +5722,24 @@ public boolean isStatementPoolingEnabled() { * */ public void setStatementPoolingCacheSize(int value) { + // Caching turned on? + String sPropKey = SQLServerDriverBooleanProperty.DISABLE_STATEMENT_POOLING.toString(); + String sPropValue = activeConnectionProperties.getProperty(sPropKey); + + // If DISABLE_STATEMENT_POOLING property is true, we can't allow cache size and will disable caching. + if ( null != sPropValue && sPropValue.equalsIgnoreCase("true")) + return; + + if (0 < value) { + preparedStatementHandleCache = new Builder() + .maximumWeightedCapacity(getStatementPoolingCacheSize()) + .listener(new PreparedStatementCacheEvictionListener()) + .build(); + + parameterMetadataCache = new Builder() + .maximumWeightedCapacity(getStatementPoolingCacheSize()) + .build(); + } if (value != this.statementPoolingCacheSize) { value = Math.max(0, value); statementPoolingCacheSize = value; @@ -5788,12 +5802,6 @@ final void evictCachedPreparedStatementHandle(PreparedStatementHandle handle) { preparedStatementHandleCache.remove(handle.getKey()); } - final void clearCachedPreparedStatementHandle() { - if (null != preparedStatementHandleCache) { - preparedStatementHandleCache.clear(); - } - } - // Handle closing handles when removed from cache. final class PreparedStatementCacheEvictionListener implements EvictionListener { public void onEviction(Sha1HashKey key, PreparedStatementHandle handle) { diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionPoolProxy.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionPoolProxy.java index 3117ec403..80d3a234d 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionPoolProxy.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionPoolProxy.java @@ -169,8 +169,6 @@ public void run() { if (wrappedConnection.getConnectionLogger().isLoggable(Level.FINER)) wrappedConnection.getConnectionLogger().finer(toString() + " Connection proxy closed "); - // clear cached prepared statement handle on this connection - wrappedConnection.clearCachedPreparedStatementHandle(); wrappedConnection.poolCloseEventNotify(); wrappedConnection = null; } diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java index bdffeab96..dd4f67aaf 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java @@ -51,8 +51,6 @@ public class SQLServerPreparedStatement extends SQLServerStatement implements ISQLServerPreparedStatement { /** Flag to indicate that it is an internal query to retrieve encryption metadata. */ boolean isInternalEncryptionQuery = false; - - boolean definitionChanged = false; /** delimiter for multiple statements in a single batch */ private static final int BATCH_STATEMENT_DELIMITER_TDS_71 = 0x80; @@ -330,14 +328,7 @@ private boolean buildPreparedStrings(Parameter[] params, boolean renewDefinition) throws SQLServerException { String newTypeDefinitions = buildParamTypeDefinitions(params, renewDefinition); if (null != preparedTypeDefinitions && newTypeDefinitions.equals(preparedTypeDefinitions)) - return false; - - if(preparedTypeDefinitions == null) { - definitionChanged = false; - } - else { - definitionChanged = true; - } + return false; preparedTypeDefinitions = newTypeDefinitions; @@ -498,8 +489,6 @@ final void processResponse(TDSReader tdsReader) throws SQLServerException { final void doExecutePreparedStatement(PrepStmtExecCmd command) throws SQLServerException { resetForReexecute(); - definitionChanged = false; - // If this request might be a query (as opposed to an update) then make // sure we set the max number of rows and max field size for any ResultSet // that may be returned. @@ -539,19 +528,32 @@ final void doExecutePreparedStatement(PrepStmtExecCmd command) throws SQLServerE } String dbName = connection.getSCatalog(); - if (reuseCachedHandle(hasNewTypeDefinitions, false, dbName)) { - hasNewTypeDefinitions = false; - } + // Retry execution if existing handle could not be re-used. + 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, dbName)) { + hasNewTypeDefinitions = false; + } - // Start the request and detach the response reader so that we can - // continue using it after we return. - TDSWriter tdsWriter = command.startRequest(TDS.PKT_RPC); + // Start the request and detach the response reader so that we can + // continue using it after we return. + TDSWriter tdsWriter = command.startRequest(TDS.PKT_RPC); - doPrepExec(tdsWriter, inOutParam, hasNewTypeDefinitions, hasExistingTypeDefinitions); + doPrepExec(tdsWriter, inOutParam, hasNewTypeDefinitions, hasExistingTypeDefinitions); - ensureExecuteResultsReader(command.startResponse(getIsResponseBufferingAdaptive())); - startResults(); - getNextResult(); + ensureExecuteResultsReader(command.startResponse(getIsResponseBufferingAdaptive())); + startResults(); + getNextResult(); + } + catch (SQLException e) { + if (retryBasedOnFailedReuseOfCachedHandle(e, attempt)) + continue; + else + throw e; + } + break; + } if (EXECUTE_QUERY == executeMethod && null == resultSet) { SQLServerException.makeFromDriverError(connection, this, SQLServerException.getErrString("R_noResultset"), null, true); @@ -560,6 +562,17 @@ else if (EXECUTE_UPDATE == executeMethod && null != resultSet) { SQLServerException.makeFromDriverError(connection, this, SQLServerException.getErrString("R_resultsetGeneratedForUpdate"), null, false); } } + + /** Should the execution be retried because the re-used cached handle could not be re-used due to server side state changes? */ + private boolean retryBasedOnFailedReuseOfCachedHandle(SQLException e, + int attempt) { + // 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. + // 99586: Error used for testing. + return 1 == attempt && (586 == e.getErrorCode() || 8179 == e.getErrorCode() || 99586 == e.getErrorCode()); + } /** * Consume the OUT parameter for the statement object itself. @@ -915,18 +928,6 @@ private void getParameterEncryptionMetadata(Parameter[] params) throws SQLServer /** Manage re-using cached handles */ private boolean reuseCachedHandle(boolean hasNewTypeDefinitions, boolean discardCurrentCacheItem, String dbName) { - if (definitionChanged || connection.contextChanged) { - prepStmtHandle = -1; // so that hasPreparedStatementHandle() also returns false - - if (connection.contextChanged) { - connection.contextChanged = false; - connection.contextIsAlreadyChanged = false; - connection.clearCachedPreparedStatementHandle(); - } - - return false; - } - // No re-use of caching for cursorable statements (statements that WILL use sp_cursor*) if (isCursorable(executeMethod)) return false; @@ -2576,7 +2577,7 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th // Create the parameter array that we'll use for all the items in this batch. Parameter[] batchParam = new Parameter[inOutParam.length]; - definitionChanged = false; + /* TDSWriter tdsWriter = null; while (numBatchesExecuted < numBatches) { @@ -2595,122 +2596,146 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th */ TDSWriter tdsWriter = null; - try { - while (numBatchesExecuted < numBatches) { - // Fill in the parameter values for this batch - Parameter paramValues[] = batchParamValues.get(numBatchesPrepared); - assert paramValues.length == batchParam.length; - System.arraycopy(paramValues, 0, batchParam, 0, paramValues.length); - - boolean hasExistingTypeDefinitions = preparedTypeDefinitions != null; - boolean hasNewTypeDefinitions = buildPreparedStrings(batchParam, false); - - // Get the encryption metadata for the first batch only. - if ((0 == numBatchesExecuted) && (Util.shouldHonorAEForParameters(stmtColumnEncriptionSetting, connection)) && (0 < batchParam.length) - && !isInternalEncryptionQuery && !encryptionMetadataIsRetrieved) { - getParameterEncryptionMetadata(batchParam); - - // fix an issue when inserting unicode into non-encrypted nchar column using setString() and AE is on on Connection - buildPreparedStrings(batchParam, true); - - // Save the crypto metadata retrieved for the first batch. We will re-use these for the rest of the batches. - for (Parameter aBatchParam : batchParam) { - cryptoMetaBatch.add(aBatchParam.cryptoMeta); - } - } + while (numBatchesExecuted < numBatches) { + // Fill in the parameter values for this batch + Parameter paramValues[] = batchParamValues.get(numBatchesPrepared); + assert paramValues.length == batchParam.length; + System.arraycopy(paramValues, 0, batchParam, 0, paramValues.length); - // Update the crypto metadata for this batch. - if (0 < numBatchesExecuted) { - // cryptoMetaBatch will be empty for non-AE connections/statements. - for (int i = 0; i < cryptoMetaBatch.size(); i++) { - batchParam[i].cryptoMeta = cryptoMetaBatch.get(i); - } - } - - String dbName = connection.getSCatalog(); - if (reuseCachedHandle(hasNewTypeDefinitions, false, dbName)) { - hasNewTypeDefinitions = false; - } + boolean hasExistingTypeDefinitions = preparedTypeDefinitions != null; + boolean hasNewTypeDefinitions = buildPreparedStrings(batchParam, false); - if (numBatchesExecuted < numBatchesPrepared) { - // assert null != tdsWriter; - tdsWriter.writeByte((byte) nBatchStatementDelimiter); + // Get the encryption metadata for the first batch only. + if ((0 == numBatchesExecuted) && (Util.shouldHonorAEForParameters(stmtColumnEncriptionSetting, connection)) && (0 < batchParam.length) + && !isInternalEncryptionQuery && !encryptionMetadataIsRetrieved) { + getParameterEncryptionMetadata(batchParam); + + // fix an issue when inserting unicode into non-encrypted nchar column using setString() and AE is on on Connection + buildPreparedStrings(batchParam, true); + + // Save the crypto metadata retrieved for the first batch. We will re-use these for the rest of the batches. + for (Parameter aBatchParam : batchParam) { + cryptoMetaBatch.add(aBatchParam.cryptoMeta); } - else { - resetForReexecute(); - tdsWriter = batchCommand.startRequest(TDS.PKT_RPC); + } + + // Update the crypto metadata for this batch. + if (0 < numBatchesExecuted) { + // cryptoMetaBatch will be empty for non-AE connections/statements. + for (int i = 0; i < cryptoMetaBatch.size(); i++) { + batchParam[i].cryptoMeta = cryptoMetaBatch.get(i); } + } - // If we have to (re)prepare the statement then we must execute it so - // that we get back a (new) prepared statement handle to use to - // execute additional batches. - // - // We must always prepare the statement the first time through. - // 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. - ++numBatchesPrepared; - - if (doPrepExec(tdsWriter, batchParam, hasNewTypeDefinitions, hasExistingTypeDefinitions) || numBatchesPrepared == numBatches) { - ensureExecuteResultsReader(batchCommand.startResponse(getIsResponseBufferingAdaptive())); - - while (numBatchesExecuted < numBatchesPrepared) { - // NOTE: - // When making changes to anything below, consider whether similar changes need - // to be made to Statement batch execution. - - startResults(); - - try { - // Get the first result from the batch. If there is no result for this batch - // then bail, leaving EXECUTE_FAILED in the current and remaining slots of - // the update count array. - if (!getNextResult()) - return; - - // If the result is a ResultSet (rather than an update count) then throw an - // exception for this result. The exception gets caught immediately below and - // translated into (or added to) a BatchUpdateException. - if (null != resultSet) { - SQLServerException.makeFromDriverError(connection, this, - SQLServerException.getErrString("R_resultsetGeneratedForUpdate"), null, false); - } - } - catch (SQLServerException e) { - // If the failure was severe enough to close the connection or roll back a - // manual transaction, then propagate the error up as a SQLServerException - // now, rather than continue with the batch. - if (connection.isSessionUnAvailable() || connection.rolledBackTransaction()) - throw e; - - // Otherwise, the connection is OK and the transaction is still intact, - // so just record the failure for the particular batch item. - updateCount = Statement.EXECUTE_FAILED; - if (null == batchCommand.batchException) - batchCommand.batchException = e; - } + String dbName = connection.getSCatalog(); + // Retry execution if existing handle could not be re-used. + for (int attempt = 1; attempt <= 2; ++attempt) { + try { - // In batch execution, we have a special update count - // to indicate that no information was returned - batchCommand.updateCounts[numBatchesExecuted] = (-1 == updateCount) ? Statement.SUCCESS_NO_INFO : updateCount; - processBatch(); + // Re-use handle if available, requires parameter definitions which are not available until here. + if (reuseCachedHandle(hasNewTypeDefinitions, 1 < attempt, dbName)) { + hasNewTypeDefinitions = false; + } - numBatchesExecuted++; + if (numBatchesExecuted < numBatchesPrepared) { + // assert null != tdsWriter; + tdsWriter.writeByte((byte) nBatchStatementDelimiter); + } + else { + resetForReexecute(); + tdsWriter = batchCommand.startRequest(TDS.PKT_RPC); } - // Only way to proceed with preparing the next set of batches is if - // we successfully executed the previously prepared set. - assert numBatchesExecuted == numBatchesPrepared; + // If we have to (re)prepare the statement then we must execute it so + // that we get back a (new) prepared statement handle to use to + // execute additional batches. + // + // We must always prepare the statement the first time through. + // 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. + ++numBatchesPrepared; + + if (doPrepExec(tdsWriter, batchParam, hasNewTypeDefinitions, hasExistingTypeDefinitions) || numBatchesPrepared == numBatches) { + ensureExecuteResultsReader(batchCommand.startResponse(getIsResponseBufferingAdaptive())); + + boolean retry = false; + while (numBatchesExecuted < numBatchesPrepared) { + // NOTE: + // When making changes to anything below, consider whether similar changes need + // to be made to Statement batch execution. + + startResults(); + + try { + // Get the first result from the batch. If there is no result for this batch + // then bail, leaving EXECUTE_FAILED in the current and remaining slots of + // the update count array. + if (!getNextResult()) + return; + + // If the result is a ResultSet (rather than an update count) then throw an + // exception for this result. The exception gets caught immediately below and + // translated into (or added to) a BatchUpdateException. + if (null != resultSet) { + SQLServerException.makeFromDriverError(connection, this, + SQLServerException.getErrString("R_resultsetGeneratedForUpdate"), null, false); + } + } + catch (SQLServerException e) { + // If the failure was severe enough to close the connection or roll back a + // manual transaction, then propagate the error up as a SQLServerException + // now, rather than continue with the batch. + if (connection.isSessionUnAvailable() || connection.rolledBackTransaction()) + throw e; + + // Retry if invalid handle exception. + if (retryBasedOnFailedReuseOfCachedHandle(e, attempt)) { + // reset number of batches prepare + numBatchesPrepared = numBatchesExecuted; + retry = true; + break; + } + + // Otherwise, the connection is OK and the transaction is still intact, + // so just record the failure for the particular batch item. + updateCount = Statement.EXECUTE_FAILED; + if (null == batchCommand.batchException) + batchCommand.batchException = e; + } + + // In batch execution, we have a special update count + // to indicate that no information was returned + batchCommand.updateCounts[numBatchesExecuted] = (-1 == updateCount) ? Statement.SUCCESS_NO_INFO : updateCount; + processBatch(); + + numBatchesExecuted++; + } + if (retry) + continue; + + // Only way to proceed with preparing the next set of batches is if + // we successfully executed the previously prepared set. + assert numBatchesExecuted == numBatchesPrepared; + } } - } - } - catch (SQLServerException e) { - // throw the initial batchException - if (null != batchCommand.batchException) { - throw batchCommand.batchException; - } - else { - throw e; + catch (SQLException e) { + if (retryBasedOnFailedReuseOfCachedHandle(e, attempt)) { + // Reset number of batches prepared. + numBatchesPrepared = numBatchesExecuted; + continue; + } + else if (null != batchCommand.batchException) { + // if batch exception occurred, loop out to throw the initial batchException + numBatchesExecuted = numBatchesPrepared; + attempt++; + continue; + } + else { + throw e; + } + } + break; } } } diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java index 696cfa6c5..f506896be 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java @@ -800,21 +800,6 @@ final void setMaxRowsAndMaxFieldSize() throws SQLServerException { } } - /* - * check if context has been changed by monitoring statment call on the connection, since context is connection based. - */ - void checkIfContextChanged(String sql) { - if (connection.contextIsAlreadyChanged) { - connection.contextChanged = true; - return; - } - else if (sql.toUpperCase().contains("ANSI_NULLS") || sql.toUpperCase().contains("QUOTED_IDENTIFIER") || sql.toUpperCase().contains("USE") - || sql.toUpperCase().contains("DEFAULT_SCHEMA")) { - connection.contextIsAlreadyChanged = true; - connection.contextChanged = true; - } - } - final void doExecuteStatement(StmtExecCmd execCmd) throws SQLServerException { resetForReexecute(); @@ -826,8 +811,6 @@ final void doExecuteStatement(StmtExecCmd execCmd) throws SQLServerException { // call syntax is rewritten here as SQL exec syntax. String sql = ensureSQLSyntax(execCmd.sql); - checkIfContextChanged(sql); - // If this request might be a query (as opposed to an update) then make // sure we set the max number of rows and max field size for any ResultSet // that may be returned. @@ -931,9 +914,6 @@ private void doExecuteStatementBatch(StmtBatchExecCmd execCmd) throws SQLServerE tdsWriter.writeString(batchIter.next()); while (batchIter.hasNext()) { tdsWriter.writeString(" ; "); - String sql = batchIter.next(); - tdsWriter.writeString(sql); - checkIfContextChanged(sql); } // Start the response 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 index 11edce8c6..20ab8ce15 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/PreparedStatementTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/PreparedStatementTest.java @@ -8,13 +8,12 @@ package com.microsoft.sqlserver.jdbc.unit.statement; import static java.util.concurrent.TimeUnit.SECONDS; -import static org.junit.jupiter.api.Assertions.assertNotSame; -import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.fail; -import java.sql.BatchUpdateException; import java.sql.DriverManager; import java.sql.ResultSet; import java.sql.SQLException; @@ -32,9 +31,9 @@ import com.microsoft.sqlserver.jdbc.SQLServerConnection; import com.microsoft.sqlserver.jdbc.SQLServerDataSource; -import com.microsoft.sqlserver.jdbc.SQLServerException; import com.microsoft.sqlserver.jdbc.SQLServerPreparedStatement; import com.microsoft.sqlserver.testframework.AbstractTest; +import com.microsoft.sqlserver.testframework.util.RandomUtil; @RunWith(JUnitPlatform.class) public class PreparedStatementTest extends AbstractTest { @@ -173,69 +172,32 @@ public void testStatementPooling() throws SQLException { } } - try (SQLServerConnection con = (SQLServerConnection) DriverManager.getConnection(connectionString)) { + try (SQLServerConnection con = (SQLServerConnection)DriverManager.getConnection(connectionString)) { // Test behvaior with statement pooling. con.setStatementPoolingCacheSize(10); - this.executeSQL(con, - "IF NOT EXISTS (SELECT * FROM sys.messages WHERE message_id = 99586) EXEC sp_addmessage 99586, 16, 'Prepared handle GAH!';"); + // 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) RAISERROR(99586,16,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) { - try { - assertSame(1, pstmt.executeUpdate()); - } - catch (SQLServerException e) { - // Error "Prepared handle GAH" is expected to happen. But it should not terminate the execution with RAISERROR. - // Since the original "Could not find prepared statement with handle" error does not terminate the execution after it. - if (!e.getMessage().contains("Prepared handle GAH")) { - throw e; - } - } + assertSame(1, pstmt.executeUpdate()); } } - // test updated value, should be 1 + 100 = 101 - // although executeUpdate() throws exception, update operation should be executed successfully. - try (ResultSet rs = con.createStatement().executeQuery("select * from #update1")) { - rs.next(); - assertSame(101, rs.getInt(1)); - } - // Test batching with missing handle failures (fake). - this.executeSQL(con, - "IF NOT EXISTS (SELECT * FROM sys.messages WHERE message_id = 99586) EXEC sp_addmessage 99586, 16, 'Prepared handle GAH!';"); 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) RAISERROR(99586,16,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) { + for (int i = 0; i < 100; ++i) pstmt.addBatch(); - } - - int[] updateCounts = null; - try { - updateCounts = pstmt.executeBatch(); - } - catch (BatchUpdateException e) { - // Error "Prepared handle GAH" is expected to happen. But it should not terminate the execution with RAISERROR. - // Since the original "Could not find prepared statement with handle" error does not terminate the execution after it. - if (!e.getMessage().contains("Prepared handle GAH")) { - throw e; - } - } - // since executeBatch() throws exception, it does not return anthing. So updateCounts is still null. - assertSame(null, updateCounts); + int[] updateCounts = pstmt.executeBatch(); - // test updated value, should be 1 + 100 = 101 - // although executeBatch() throws exception, update operation should be executed successfully. - try (ResultSet rs = con.createStatement().executeQuery("select * from #update2")) { - rs.next(); - assertSame(101, rs.getInt(1)); + // Verify update counts are correct + for (int i : updateCounts) { + assertSame(1, i); } } } @@ -304,7 +266,6 @@ public void testStatementPoolingEviction() throws SQLException { for (int testNo = 0; testNo < 2; ++testNo) { try (SQLServerConnection con = (SQLServerConnection)DriverManager.getConnection(connectionString)) { - int cacheSize = 10; int discardedStatementCount = testNo == 0 ? 5 /*batched unprepares*/ : 0 /*regular unprepares*/; @@ -452,7 +413,7 @@ public void testStatementPoolingPreparedStatementExecAndUnprepareConfig() throws SQLServerDataSource dataSource = new SQLServerDataSource(); dataSource.setURL(connectionString); // Verify defaults. - assertTrue(0 < dataSource.getStatementPoolingCacheSize()); + assertTrue(0 == dataSource.getStatementPoolingCacheSize()); // Verify change dataSource.setStatementPoolingCacheSize(0); assertSame(0, dataSource.getStatementPoolingCacheSize()); @@ -469,11 +430,14 @@ public void testStatementPoolingPreparedStatementExecAndUnprepareConfig() throws // Test disableStatementPooling String connectionStringDisableStatementPooling = connectionString + ";disableStatementPooling=true;"; SQLServerConnection connectionDisableStatementPooling = (SQLServerConnection)DriverManager.getConnection(connectionStringDisableStatementPooling); + connectionDisableStatementPooling.setStatementPoolingCacheSize(10); // to turn on caching and check if disableStatementPooling is true, even setting cachesize won't matter and will disable it. assertSame(0, connectionDisableStatementPooling.getStatementPoolingCacheSize()); assertTrue(!connectionDisableStatementPooling.isStatementPoolingEnabled()); String connectionStringEnableStatementPooling = connectionString + ";disableStatementPooling=false;"; SQLServerConnection connectionEnableStatementPooling = (SQLServerConnection)DriverManager.getConnection(connectionStringEnableStatementPooling); - assertTrue(0 < connectionEnableStatementPooling.getStatementPoolingCacheSize()); + connectionEnableStatementPooling.setStatementPoolingCacheSize(10); // to turn on caching. + assertTrue(0 < connectionEnableStatementPooling.getStatementPoolingCacheSize()); // for now, it won't affect if disable is false or true. Since statementPoolingCacheSize is set to 0 as default. + //If only disableStatementPooling is set to true, it makes sure that statementPoolingCacheSize is zero, thus disabling the prepared statement metadata caching. // Test EnablePrepareOnFirstPreparedStatementCall String connectionStringNoExecuteSQL = connectionString + ";enablePrepareOnFirstPreparedStatementCall=true;"; @@ -545,54 +509,4 @@ public void testStatementPoolingPreparedStatementExecAndUnprepareConfig() throws assertSame(0, con.getDiscardedServerPreparedStatementCount()); } } - - /** - * Validate the right behavior for EnablePrepareOnFirstPreparedStatementCall with - * statement pooling turned off. - * - * @throws SQLException - */ - @Test - public void testEnablePrepareOnFirstPreparedStatementCallWithStatementPoolingOff() throws SQLException { - - try (SQLServerConnection con = (SQLServerConnection)DriverManager.getConnection(connectionString)) { - - // Turn off use of prepared statement cache. - con.setStatementPoolingCacheSize(0); - // Disable EnablePrepareOnFirstPreparedStatementCall (default) - con.setEnablePrepareOnFirstPreparedStatementCall(false); - - String query = "/*testEnablePrepareOnFirstPreparedStatementCallWithStatementPoolingOff*/SELECT * FROM sys.objects;"; - - // Verify first use is never prepared. - for(int i = 0; i < 10; ++i) - { - try (SQLServerPreparedStatement pstmt = (SQLServerPreparedStatement)con.prepareStatement(query)) { - pstmt.execute(); // sp_executesql - pstmt.getMoreResults(); // Make sure handle is updated. - - // Validate no handle was created. - assertTrue(0 >= pstmt.getPreparedStatementHandle()); - } - } - - // Verify second use is prepared. - for(int i = 0; i < 10; ++i) - { - try (SQLServerPreparedStatement pstmt = (SQLServerPreparedStatement)con.prepareStatement(query)) { - pstmt.execute(); // sp_executesql - pstmt.getMoreResults(); // Make sure handle is updated. - - // Validate no handle was created. - assertTrue(0 >= pstmt.getPreparedStatementHandle()); - - pstmt.execute(); // sp_prepexec - pstmt.getMoreResults(); // Make sure handle is updated. - - // Validate handle was created. - assertTrue(0 < pstmt.getPreparedStatementHandle()); - } - } - } - } -} +} \ No newline at end of file From 69edc7b2e59145e4697576e9ca4b77fb9e6b77c6 Mon Sep 17 00:00:00 2001 From: Afsaneh Rafighi Date: Tue, 30 Jan 2018 13:59:18 -0800 Subject: [PATCH 02/12] check if statementPooling is enabled for all retry logics --- .../sqlserver/jdbc/SQLServerPreparedStatement.java | 9 +++++---- .../com/microsoft/sqlserver/jdbc/SQLServerStatement.java | 1 + 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java index dd4f67aaf..4c2b22037 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java @@ -547,7 +547,7 @@ final void doExecutePreparedStatement(PrepStmtExecCmd command) throws SQLServerE getNextResult(); } catch (SQLException e) { - if (retryBasedOnFailedReuseOfCachedHandle(e, attempt)) + if (retryBasedOnFailedReuseOfCachedHandle(e, attempt) && connection.isStatementPoolingEnabled()) continue; else throw e; @@ -2690,7 +2690,7 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th throw e; // Retry if invalid handle exception. - if (retryBasedOnFailedReuseOfCachedHandle(e, attempt)) { + if (retryBasedOnFailedReuseOfCachedHandle(e, attempt) && connection.isStatementPoolingEnabled()) { // reset number of batches prepare numBatchesPrepared = numBatchesExecuted; retry = true; @@ -2701,7 +2701,8 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th // so just record the failure for the particular batch item. updateCount = Statement.EXECUTE_FAILED; if (null == batchCommand.batchException) - batchCommand.batchException = e; + batchCommand.batchException = e; + } // In batch execution, we have a special update count @@ -2720,7 +2721,7 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th } } catch (SQLException e) { - if (retryBasedOnFailedReuseOfCachedHandle(e, attempt)) { + if (retryBasedOnFailedReuseOfCachedHandle(e, attempt) && connection.isStatementPoolingEnabled()) { // Reset number of batches prepared. numBatchesPrepared = numBatchesExecuted; continue; diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java index f506896be..7d7608433 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java @@ -914,6 +914,7 @@ private void doExecuteStatementBatch(StmtBatchExecCmd execCmd) throws SQLServerE tdsWriter.writeString(batchIter.next()); while (batchIter.hasNext()) { tdsWriter.writeString(" ; "); + tdsWriter.writeString(batchIter.next()); } // Start the response From b8e9ff9007c72f32e9307931ec4ed1634a785d12 Mon Sep 17 00:00:00 2001 From: Afsaneh Rafighi Date: Wed, 31 Jan 2018 12:44:36 -0800 Subject: [PATCH 03/12] add the setter and getter methods for disableStatementPooling property --- .../sqlserver/jdbc/SQLServerConnection.java | 82 ++++++++++++------- .../sqlserver/jdbc/SQLServerDataSource.java | 22 ++++- .../sqlserver/jdbc/SQLServerDriver.java | 2 +- .../unit/statement/PreparedStatementTest.java | 8 +- 4 files changed, 79 insertions(+), 35 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index edfa63a20..d4f2b02ce 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -283,6 +283,10 @@ static ParsedSQLCacheItem parseAndCacheSQL(Sha1HashKey key, String sql) throws private ConcurrentLinkedHashMap preparedStatementHandleCache; /** Cache of prepared statement parameter metadata */ private ConcurrentLinkedHashMap parameterMetadataCache; + /** + * Checks whether statement pooling is enabled or disabled. The default is set to true; + */ + private boolean disableStatementPooling = true; /** * Find statement parameters. @@ -923,9 +927,9 @@ final boolean attachConnId() { connectionlogger.severe(message); throw new UnsupportedOperationException(message); } - + // Caching turned on? - if (0 < this.getStatementPoolingCacheSize()) { + if (!this.getDisableStatementPooling() && 0 < this.getStatementPoolingCacheSize() ) { preparedStatementHandleCache = new Builder() .maximumWeightedCapacity(getStatementPoolingCacheSize()) .listener(new PreparedStatementCacheEvictionListener()) @@ -1436,9 +1440,11 @@ Connection connectInternal(Properties propsIn, sPropValue = activeConnectionProperties.getProperty(sPropKey); if (null != sPropValue) { // If disabled set cache size to 0 if disabled. - if(booleanPropertyOn(sPropKey, sPropValue)) - this.setStatementPoolingCacheSize(0); - } + if(booleanPropertyOn(sPropKey, sPropValue)) { + this.setStatementPoolingCacheSize(0); + } + disableStatementPooling = booleanPropertyOn(sPropKey, sPropValue); + } sPropKey = SQLServerDriverBooleanProperty.INTEGRATED_SECURITY.toString(); sPropValue = activeConnectionProperties.getProperty(sPropKey); @@ -5688,6 +5694,23 @@ final void unprepareUnreferencedPreparedStatementHandles(boolean force) { } } + /** + * Returns true if statement pooling is disabled. + * + * @return + */ + public boolean getDisableStatementPooling() { + return this.disableStatementPooling; + } + + /** + * Sets statement pooling to true or false; + * + * @param value + */ + public void setDisableStatementPooling(boolean value) { + this.disableStatementPooling = value; + } /** * Returns the size of the prepared statement cache for this connection. A value less than 1 means no cache. @@ -5717,38 +5740,37 @@ public boolean isStatementPoolingEnabled() { } /** - * Specifies the size of the prepared statement cache for this conection. A value less than 1 means no cache. - * @param value The new cache size. + * Specifies the size of the prepared statement cache for this connection. A value less than 1 means no cache. + * + * @param value + * The new cache size. * */ public void setStatementPoolingCacheSize(int value) { - // Caching turned on? - String sPropKey = SQLServerDriverBooleanProperty.DISABLE_STATEMENT_POOLING.toString(); - String sPropValue = activeConnectionProperties.getProperty(sPropKey); - - // If DISABLE_STATEMENT_POOLING property is true, we can't allow cache size and will disable caching. - if ( null != sPropValue && sPropValue.equalsIgnoreCase("true")) - return; - - if (0 < value) { - preparedStatementHandleCache = new Builder() - .maximumWeightedCapacity(getStatementPoolingCacheSize()) - .listener(new PreparedStatementCacheEvictionListener()) - .build(); + value = Math.max(0, value); + statementPoolingCacheSize = value; - parameterMetadataCache = new Builder() - .maximumWeightedCapacity(getStatementPoolingCacheSize()) - .build(); + if (!this.disableStatementPooling) { + prepareCache(value); } - if (value != this.statementPoolingCacheSize) { - value = Math.max(0, value); - statementPoolingCacheSize = value; + if (null != preparedStatementHandleCache) + preparedStatementHandleCache.setCapacity(value); + + if (null != parameterMetadataCache) + parameterMetadataCache.setCapacity(value); + } - if (null != preparedStatementHandleCache) - preparedStatementHandleCache.setCapacity(value); + /** + * Internal method to prepare the cache handle + * @param value + */ + private void prepareCache(int value) { + if (0 < value) { + preparedStatementHandleCache = new Builder().maximumWeightedCapacity(getStatementPoolingCacheSize()) + .listener(new PreparedStatementCacheEvictionListener()).build(); - if (null != parameterMetadataCache) - parameterMetadataCache.setCapacity(value); + parameterMetadataCache = new Builder().maximumWeightedCapacity(getStatementPoolingCacheSize()) + .build(); } } diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java index ebe4fee35..2f354f1c4 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java @@ -744,7 +744,7 @@ public int getServerPreparedStatementDiscardThreshold() { } /** - * Specifies the size of the prepared statement cache for this conection. A value less than 1 means no cache. + * Specifies the size of the prepared statement cache for this connection. A value less than 1 means no cache. * * @param statementPoolingCacheSize * Changes the setting per the description. @@ -754,7 +754,7 @@ public void setStatementPoolingCacheSize(int statementPoolingCacheSize) { } /** - * Returns the size of the prepared statement cache for this conection. A value less than 1 means no cache. + * Returns the size of the prepared statement cache for this connection. A value less than 1 means no cache. * * @return Returns the current setting per the description. */ @@ -762,6 +762,24 @@ public int getStatementPoolingCacheSize() { int defaultSize = SQLServerDriverIntProperty.STATEMENT_POOLING_CACHE_SIZE.getDefaultValue(); return getIntProperty(connectionProps, SQLServerDriverIntProperty.STATEMENT_POOLING_CACHE_SIZE.toString(), defaultSize); } + + /** + * Sets the statement pooling to true or false + * @param disableStatementPooling + */ + public void setDisableStatementPooling(boolean disableStatementPooling) { + setBooleanProperty(connectionProps, SQLServerDriverBooleanProperty.DISABLE_STATEMENT_POOLING.toString(), disableStatementPooling); + } + + /** + * Returns true if statement pooling is disabled. + * @return + */ + public boolean getDisableStatementPooling() { + boolean defaultValue = SQLServerDriverBooleanProperty.DISABLE_STATEMENT_POOLING.getDefaultValue(); + return getBooleanProperty(connectionProps, SQLServerDriverBooleanProperty.DISABLE_STATEMENT_POOLING.toString(), + defaultValue); + } /** * Setting the socket timeout diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java index ae4e1fd09..e9e94a5ec 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java @@ -340,7 +340,7 @@ public String toString() { enum SQLServerDriverBooleanProperty { - DISABLE_STATEMENT_POOLING ("disableStatementPooling", false), + DISABLE_STATEMENT_POOLING ("disableStatementPooling", true), ENCRYPT ("encrypt", false), INTEGRATED_SECURITY ("integratedSecurity", false), LAST_UPDATE_COUNT ("lastUpdateCount", true), 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 index 20ab8ce15..9fba106dc 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/PreparedStatementTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/PreparedStatementTest.java @@ -175,6 +175,7 @@ public void testStatementPooling() throws SQLException { try (SQLServerConnection con = (SQLServerConnection)DriverManager.getConnection(connectionString)) { // Test behvaior with statement pooling. + con.setDisableStatementPooling(false); con.setStatementPoolingCacheSize(10); // Test with missing handle failures (fake). @@ -204,6 +205,7 @@ public void testStatementPooling() throws SQLException { try (SQLServerConnection con = (SQLServerConnection)DriverManager.getConnection(connectionString)) { // Test behvaior with statement pooling. + con.setDisableStatementPooling(false); con.setStatementPoolingCacheSize(10); String lookupUniqueifier = UUID.randomUUID().toString(); @@ -269,7 +271,9 @@ public void testStatementPoolingEviction() throws SQLException { int cacheSize = 10; int discardedStatementCount = testNo == 0 ? 5 /*batched unprepares*/ : 0 /*regular unprepares*/; - con.setStatementPoolingCacheSize(cacheSize); + // enabling caching + con.setDisableStatementPooling(false); + con.setStatementPoolingCacheSize(cacheSize); con.setServerPreparedStatementDiscardThreshold(discardedStatementCount); String lookupUniqueifier = UUID.randomUUID().toString(); @@ -431,7 +435,7 @@ public void testStatementPoolingPreparedStatementExecAndUnprepareConfig() throws String connectionStringDisableStatementPooling = connectionString + ";disableStatementPooling=true;"; SQLServerConnection connectionDisableStatementPooling = (SQLServerConnection)DriverManager.getConnection(connectionStringDisableStatementPooling); connectionDisableStatementPooling.setStatementPoolingCacheSize(10); // to turn on caching and check if disableStatementPooling is true, even setting cachesize won't matter and will disable it. - assertSame(0, connectionDisableStatementPooling.getStatementPoolingCacheSize()); + assertSame(10, connectionDisableStatementPooling.getStatementPoolingCacheSize()); assertTrue(!connectionDisableStatementPooling.isStatementPoolingEnabled()); String connectionStringEnableStatementPooling = connectionString + ";disableStatementPooling=false;"; SQLServerConnection connectionEnableStatementPooling = (SQLServerConnection)DriverManager.getConnection(connectionStringEnableStatementPooling); From af76bfc5968e36775864fe4e310a4a99a7147351 Mon Sep 17 00:00:00 2001 From: Afsaneh Rafighi Date: Wed, 31 Jan 2018 12:53:08 -0800 Subject: [PATCH 04/12] move same code in a function. --- .../sqlserver/jdbc/SQLServerConnection.java | 27 +++++++------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index d4f2b02ce..d5a77247c 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -929,15 +929,8 @@ final boolean attachConnId() { } // Caching turned on? - if (!this.getDisableStatementPooling() && 0 < this.getStatementPoolingCacheSize() ) { - preparedStatementHandleCache = new Builder() - .maximumWeightedCapacity(getStatementPoolingCacheSize()) - .listener(new PreparedStatementCacheEvictionListener()) - .build(); - - parameterMetadataCache = new Builder() - .maximumWeightedCapacity(getStatementPoolingCacheSize()) - .build(); + if (!this.getDisableStatementPooling() && 0 < this.getStatementPoolingCacheSize()) { + prepareCache(); } } @@ -5750,8 +5743,8 @@ public void setStatementPoolingCacheSize(int value) { value = Math.max(0, value); statementPoolingCacheSize = value; - if (!this.disableStatementPooling) { - prepareCache(value); + if (!this.disableStatementPooling && value > 0) { + prepareCache(); } if (null != preparedStatementHandleCache) preparedStatementHandleCache.setCapacity(value); @@ -5764,14 +5757,12 @@ public void setStatementPoolingCacheSize(int value) { * Internal method to prepare the cache handle * @param value */ - private void prepareCache(int value) { - if (0 < value) { - preparedStatementHandleCache = new Builder().maximumWeightedCapacity(getStatementPoolingCacheSize()) - .listener(new PreparedStatementCacheEvictionListener()).build(); + private void prepareCache() { + preparedStatementHandleCache = new Builder().maximumWeightedCapacity(getStatementPoolingCacheSize()) + .listener(new PreparedStatementCacheEvictionListener()).build(); - parameterMetadataCache = new Builder().maximumWeightedCapacity(getStatementPoolingCacheSize()) - .build(); - } + parameterMetadataCache = new Builder().maximumWeightedCapacity(getStatementPoolingCacheSize()) + .build(); } /** Get a parameter metadata cache entry if statement pooling is enabled */ From a237ebeb17a738b2a6bda2d0e6e66eb69ab55b0f Mon Sep 17 00:00:00 2001 From: Afsaneh Rafighi Date: Wed, 31 Jan 2018 13:21:33 -0800 Subject: [PATCH 05/12] prepare cache in setDisableStatementPooling() --- .../com/microsoft/sqlserver/jdbc/SQLServerConnection.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index d5a77247c..121886f2f 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -275,7 +275,7 @@ static ParsedSQLCacheItem parseAndCacheSQL(Sha1HashKey key, String sql) throws } /** Size of the prepared statement handle cache */ - private int statementPoolingCacheSize = 0; + private int statementPoolingCacheSize = DEFAULT_STATEMENT_POOLING_CACHE_SIZE; /** Default size for prepared statement caches */ static final int DEFAULT_STATEMENT_POOLING_CACHE_SIZE = 0; @@ -5703,6 +5703,9 @@ public boolean getDisableStatementPooling() { */ public void setDisableStatementPooling(boolean value) { this.disableStatementPooling = value; + if (!value && 0 < this.getStatementPoolingCacheSize()) { + prepareCache(); + } } /** @@ -5729,7 +5732,7 @@ public int getStatementHandleCacheEntryCount() { * @return Returns the current setting per the description. */ public boolean isStatementPoolingEnabled() { - return null != preparedStatementHandleCache && 0 < this.getStatementPoolingCacheSize(); + return null != preparedStatementHandleCache && 0 < this.getStatementPoolingCacheSize() && !this.getDisableStatementPooling(); } /** From 63ab78ffe9ecad52c763d43870645bd2a108710b Mon Sep 17 00:00:00 2001 From: Afsaneh Rafighi Date: Wed, 31 Jan 2018 13:45:36 -0800 Subject: [PATCH 06/12] a small fix --- .../com/microsoft/sqlserver/jdbc/SQLServerConnection.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index 121886f2f..c9e4b4ae5 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -274,11 +274,12 @@ static ParsedSQLCacheItem parseAndCacheSQL(Sha1HashKey key, String sql) throws return cacheItem; } + /** Default size for prepared statement caches */ + static final int DEFAULT_STATEMENT_POOLING_CACHE_SIZE = 0; + /** Size of the prepared statement handle cache */ private int statementPoolingCacheSize = DEFAULT_STATEMENT_POOLING_CACHE_SIZE; - /** Default size for prepared statement caches */ - static final int DEFAULT_STATEMENT_POOLING_CACHE_SIZE = 0; /** Cache of prepared statement handles */ private ConcurrentLinkedHashMap preparedStatementHandleCache; /** Cache of prepared statement parameter metadata */ From dbefa1cde4568985cfc819c14c50110a3f4276af Mon Sep 17 00:00:00 2001 From: Afsaneh Rafighi Date: Wed, 31 Jan 2018 15:28:36 -0800 Subject: [PATCH 07/12] fixed the double insertion issue with retry --- .../jdbc/SQLServerPreparedStatement.java | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java index 4c2b22037..c509d7db8 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java @@ -528,6 +528,7 @@ final void doExecutePreparedStatement(PrepStmtExecCmd command) throws SQLServerE } String dbName = connection.getSCatalog(); + boolean needsPrepare = false; // Retry execution if existing handle could not be re-used. for (int attempt = 1; attempt <= 2; ++attempt) { try { @@ -540,14 +541,14 @@ final void doExecutePreparedStatement(PrepStmtExecCmd command) throws SQLServerE // continue using it after we return. TDSWriter tdsWriter = command.startRequest(TDS.PKT_RPC); - doPrepExec(tdsWriter, inOutParam, hasNewTypeDefinitions, hasExistingTypeDefinitions); + needsPrepare = doPrepExec(tdsWriter, inOutParam, hasNewTypeDefinitions, hasExistingTypeDefinitions); ensureExecuteResultsReader(command.startResponse(getIsResponseBufferingAdaptive())); startResults(); getNextResult(); } catch (SQLException e) { - if (retryBasedOnFailedReuseOfCachedHandle(e, attempt) && connection.isStatementPoolingEnabled()) + if (retryBasedOnFailedReuseOfCachedHandle(e, attempt, needsPrepare) && connection.isStatementPoolingEnabled()) continue; else throw e; @@ -565,13 +566,18 @@ else if (EXECUTE_UPDATE == executeMethod && null != resultSet) { /** Should the execution be retried because the re-used cached handle could not be re-used due to server side state changes? */ private boolean retryBasedOnFailedReuseOfCachedHandle(SQLException e, - int attempt) { + int attempt, boolean needsPrepare) { // 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. // 99586: Error used for testing. - return 1 == attempt && (586 == e.getErrorCode() || 8179 == e.getErrorCode() || 99586 == e.getErrorCode()); + if (needsPrepare) { + return false; + } + else { + return 1 == attempt && (586 == e.getErrorCode() || 8179 == e.getErrorCode() || 99586 == e.getErrorCode()); + } } /** @@ -2628,6 +2634,7 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th } String dbName = connection.getSCatalog(); + boolean needsPrepare = false; // Retry execution if existing handle could not be re-used. for (int attempt = 1; attempt <= 2; ++attempt) { try { @@ -2655,8 +2662,8 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th // the size of a batch's string parameter values changes such // that repreparation is necessary. ++numBatchesPrepared; - - if (doPrepExec(tdsWriter, batchParam, hasNewTypeDefinitions, hasExistingTypeDefinitions) || numBatchesPrepared == numBatches) { + needsPrepare = doPrepExec(tdsWriter, batchParam, hasNewTypeDefinitions, hasExistingTypeDefinitions); + if ( needsPrepare || numBatchesPrepared == numBatches) { ensureExecuteResultsReader(batchCommand.startResponse(getIsResponseBufferingAdaptive())); boolean retry = false; @@ -2690,7 +2697,7 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th throw e; // Retry if invalid handle exception. - if (retryBasedOnFailedReuseOfCachedHandle(e, attempt) && connection.isStatementPoolingEnabled()) { + if (retryBasedOnFailedReuseOfCachedHandle(e, attempt, needsPrepare) && connection.isStatementPoolingEnabled()) { // reset number of batches prepare numBatchesPrepared = numBatchesExecuted; retry = true; @@ -2721,7 +2728,7 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th } } catch (SQLException e) { - if (retryBasedOnFailedReuseOfCachedHandle(e, attempt) && connection.isStatementPoolingEnabled()) { + if (retryBasedOnFailedReuseOfCachedHandle(e, attempt, needsPrepare) && connection.isStatementPoolingEnabled()) { // Reset number of batches prepared. numBatchesPrepared = numBatchesExecuted; continue; From 870e853cdfc795db58738d8e2a6c8ea2498d4ad1 Mon Sep 17 00:00:00 2001 From: Afsaneh Rafighi Date: Wed, 31 Jan 2018 17:34:58 -0800 Subject: [PATCH 08/12] fixed some review comments and updated test to use raiseError --- .../jdbc/SQLServerPreparedStatement.java | 8 +-- .../unit/statement/PreparedStatementTest.java | 61 +++++++++++++++---- 2 files changed, 53 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java index c509d7db8..d4fe81010 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java @@ -548,7 +548,7 @@ final void doExecutePreparedStatement(PrepStmtExecCmd command) throws SQLServerE getNextResult(); } catch (SQLException e) { - if (retryBasedOnFailedReuseOfCachedHandle(e, attempt, needsPrepare) && connection.isStatementPoolingEnabled()) + if (retryBasedOnFailedReuseOfCachedHandle(e, attempt, needsPrepare)) continue; else throw e; @@ -567,7 +567,7 @@ else if (EXECUTE_UPDATE == executeMethod && null != resultSet) { /** Should the execution be retried because the re-used cached handle could not be re-used due to server side state changes? */ private boolean retryBasedOnFailedReuseOfCachedHandle(SQLException e, int attempt, boolean needsPrepare) { - // Only retry based on these error codes: + // Only retry based on these error codes and if statementPooling is enabled: // 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. @@ -576,7 +576,7 @@ private boolean retryBasedOnFailedReuseOfCachedHandle(SQLException e, return false; } else { - return 1 == attempt && (586 == e.getErrorCode() || 8179 == e.getErrorCode() || 99586 == e.getErrorCode()); + return 1 == attempt && (586 == e.getErrorCode() || 8179 == e.getErrorCode() || 99586 == e.getErrorCode()) && connection.isStatementPoolingEnabled(); } } @@ -2697,7 +2697,7 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th throw e; // Retry if invalid handle exception. - if (retryBasedOnFailedReuseOfCachedHandle(e, attempt, needsPrepare) && connection.isStatementPoolingEnabled()) { + if (retryBasedOnFailedReuseOfCachedHandle(e, attempt, needsPrepare)) { // reset number of batches prepare numBatchesPrepared = numBatchesExecuted; retry = true; 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 index 9fba106dc..22d883908 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/PreparedStatementTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/PreparedStatementTest.java @@ -14,6 +14,7 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.fail; +import java.sql.BatchUpdateException; import java.sql.DriverManager; import java.sql.ResultSet; import java.sql.SQLException; @@ -31,9 +32,9 @@ import com.microsoft.sqlserver.jdbc.SQLServerConnection; import com.microsoft.sqlserver.jdbc.SQLServerDataSource; +import com.microsoft.sqlserver.jdbc.SQLServerException; import com.microsoft.sqlserver.jdbc.SQLServerPreparedStatement; import com.microsoft.sqlserver.testframework.AbstractTest; -import com.microsoft.sqlserver.testframework.util.RandomUtil; @RunWith(JUnitPlatform.class) public class PreparedStatementTest extends AbstractTest { @@ -175,34 +176,70 @@ public void testStatementPooling() throws SQLException { try (SQLServerConnection con = (SQLServerConnection)DriverManager.getConnection(connectionString)) { // Test behvaior with statement pooling. - con.setDisableStatementPooling(false); con.setStatementPoolingCacheSize(10); - + this.executeSQL(con, + "IF NOT EXISTS (SELECT * FROM sys.messages WHERE message_id = 99586) EXEC sp_addmessage 99586, 16, 'Prepared handle GAH!';"); // 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;"); + this.executeSQL(con, + "CREATE PROC #updateProc1 AS UPDATE #update1 SET col += 1; IF EXISTS (SELECT * FROM #update1 WHERE col % 5 = 0) RAISERROR(99586,16,1);"); try (SQLServerPreparedStatement pstmt = (SQLServerPreparedStatement) con.prepareStatement("#updateProc1")) { for (int i = 0; i < 100; ++i) { - assertSame(1, pstmt.executeUpdate()); + try { + assertSame(1, pstmt.executeUpdate()); + } + catch (SQLServerException e) { + // Error "Prepared handle GAH" is expected to happen. But it should not terminate the execution with RAISERROR. + // Since the original "Could not find prepared statement with handle" error does not terminate the execution after it. + if (!e.getMessage().contains("Prepared handle GAH")) { + throw e; + } + } } } + // test updated value, should be 1 + 100 = 101 + // although executeUpdate() throws exception, update operation should be executed successfully. + try (ResultSet rs = con.createStatement().executeQuery("select * from #update1")) { + rs.next(); + assertSame(101, rs.getInt(1)); + } + // Test batching with missing handle failures (fake). + this.executeSQL(con, + "IF NOT EXISTS (SELECT * FROM sys.messages WHERE message_id = 99586) EXEC sp_addmessage 99586, 16, 'Prepared handle GAH!';"); 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;"); + this.executeSQL(con, + "CREATE PROC #updateProc2 AS UPDATE #update2 SET col += 1; IF EXISTS (SELECT * FROM #update2 WHERE col % 5 = 0) RAISERROR(99586,16,1);"); try (SQLServerPreparedStatement pstmt = (SQLServerPreparedStatement) con.prepareStatement("#updateProc2")) { - for (int i = 0; i < 100; ++i) + for (int i = 0; i < 100; ++i) { pstmt.addBatch(); + } - int[] updateCounts = pstmt.executeBatch(); + int[] updateCounts = null; + try { + updateCounts = pstmt.executeBatch(); + } + catch (BatchUpdateException e) { + // Error "Prepared handle GAH" is expected to happen. But it should not terminate the execution with RAISERROR. + // Since the original "Could not find prepared statement with handle" error does not terminate the execution after it. + if (!e.getMessage().contains("Prepared handle GAH")) { + throw e; + } + } - // Verify update counts are correct - for (int i : updateCounts) { - assertSame(1, i); + // since executeBatch() throws exception, it does not return anthing. So updateCounts is still null. + assertSame(null, updateCounts); + + // test updated value, should be 1 + 100 = 101 + // although executeBatch() throws exception, update operation should be executed successfully. + try (ResultSet rs = con.createStatement().executeQuery("select * from #update2")) { + rs.next(); + assertSame(101, rs.getInt(1)); } } } - + try (SQLServerConnection con = (SQLServerConnection)DriverManager.getConnection(connectionString)) { // Test behvaior with statement pooling. con.setDisableStatementPooling(false); From f523ca90cc0aace7564714d52e55373d1c57aeba Mon Sep 17 00:00:00 2001 From: Afsaneh Rafighi Date: Thu, 1 Feb 2018 14:43:26 -0800 Subject: [PATCH 09/12] added a fix with connection property --- .../com/microsoft/sqlserver/jdbc/SQLServerConnection.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index c9e4b4ae5..87230f471 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -1437,8 +1437,8 @@ Connection connectInternal(Properties propsIn, if(booleanPropertyOn(sPropKey, sPropValue)) { this.setStatementPoolingCacheSize(0); } - disableStatementPooling = booleanPropertyOn(sPropKey, sPropValue); - } + setDisableStatementPooling(booleanPropertyOn(sPropKey, sPropValue)); + } sPropKey = SQLServerDriverBooleanProperty.INTEGRATED_SECURITY.toString(); sPropValue = activeConnectionProperties.getProperty(sPropKey); From 6ed48e2b61f386b1d7b5806974d7cbb223b88f01 Mon Sep 17 00:00:00 2001 From: Afsaneh Rafighi Date: Thu, 1 Feb 2018 15:37:13 -0800 Subject: [PATCH 10/12] added unit Tests --- .../sqlserver/jdbc/SQLServerConnection.java | 6 +----- .../unit/statement/PreparedStatementTest.java | 21 ++++++++++++++++++- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index 87230f471..432aa9e32 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -1433,12 +1433,8 @@ Connection connectInternal(Properties propsIn, sPropKey = SQLServerDriverBooleanProperty.DISABLE_STATEMENT_POOLING.toString(); sPropValue = activeConnectionProperties.getProperty(sPropKey); if (null != sPropValue) { - // If disabled set cache size to 0 if disabled. - if(booleanPropertyOn(sPropKey, sPropValue)) { - this.setStatementPoolingCacheSize(0); - } setDisableStatementPooling(booleanPropertyOn(sPropKey, sPropValue)); - } + } sPropKey = SQLServerDriverBooleanProperty.INTEGRATED_SECURITY.toString(); sPropValue = activeConnectionProperties.getProperty(sPropKey); 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 index 22d883908..58c7969ab 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/PreparedStatementTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/PreparedStatementTest.java @@ -479,7 +479,26 @@ public void testStatementPoolingPreparedStatementExecAndUnprepareConfig() throws connectionEnableStatementPooling.setStatementPoolingCacheSize(10); // to turn on caching. assertTrue(0 < connectionEnableStatementPooling.getStatementPoolingCacheSize()); // for now, it won't affect if disable is false or true. Since statementPoolingCacheSize is set to 0 as default. //If only disableStatementPooling is set to true, it makes sure that statementPoolingCacheSize is zero, thus disabling the prepared statement metadata caching. - + assertTrue(connectionEnableStatementPooling.isStatementPoolingEnabled()); + + String connectionPropertyStringEnableStatementPooling = connectionString + ";disableStatementPooling=false;statementPoolingCacheSize=10"; + SQLServerConnection connectionPropertyEnableStatementPooling = (SQLServerConnection)DriverManager.getConnection(connectionPropertyStringEnableStatementPooling); + assertTrue(0 < connectionPropertyEnableStatementPooling.getStatementPoolingCacheSize()); // for now, it won't affect if disable is false or true. Since statementPoolingCacheSize is set to 0 as default. + //If only disableStatementPooling is set to true, it makes sure that statementPoolingCacheSize is zero, thus disabling the prepared statement metadata caching. + assertTrue(connectionPropertyEnableStatementPooling.isStatementPoolingEnabled()); + + String connectionPropertyStringDisableStatementPooling = connectionString + ";disableStatementPooling=true;statementPoolingCacheSize=10"; + SQLServerConnection connectionPropertyDisableStatementPooling = (SQLServerConnection)DriverManager.getConnection(connectionPropertyStringDisableStatementPooling); + assertTrue(0 < connectionPropertyDisableStatementPooling.getStatementPoolingCacheSize()); // for now, it won't affect if disable is false or true. Since statementPoolingCacheSize is set to 0 as default. + //If only disableStatementPooling is set to true, it makes sure that statementPoolingCacheSize is zero, thus disabling the prepared statement metadata caching. + assertTrue(!connectionPropertyDisableStatementPooling.isStatementPoolingEnabled()); + + String connectionPropertyStringDisableStatementPooling2 = connectionString + ";disableStatementPooling=false;statementPoolingCacheSize=0"; + SQLServerConnection connectionPropertyDisableStatementPooling2 = (SQLServerConnection)DriverManager.getConnection(connectionPropertyStringDisableStatementPooling2); + assertTrue(0 == connectionPropertyDisableStatementPooling2.getStatementPoolingCacheSize()); // for now, it won't affect if disable is false or true. Since statementPoolingCacheSize is set to 0 as default. + //If only disableStatementPooling is set to true, it makes sure that statementPoolingCacheSize is zero, thus disabling the prepared statement metadata caching. + assertTrue(!connectionPropertyDisableStatementPooling2.isStatementPoolingEnabled()); + // Test EnablePrepareOnFirstPreparedStatementCall String connectionStringNoExecuteSQL = connectionString + ";enablePrepareOnFirstPreparedStatementCall=true;"; SQLServerConnection connectionNoExecuteSQL = (SQLServerConnection)DriverManager.getConnection(connectionStringNoExecuteSQL); From e221b321984b517c876a2aecbcc29a678c2accbe Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Wed, 14 Feb 2018 13:19:02 -0800 Subject: [PATCH 11/12] Revert back needsPrepare check in retryBasedOnFailedReuseOfCachedHandle --- .../sqlserver/jdbc/SQLServerPreparedStatement.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java index d4fe81010..b133cc67a 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java @@ -572,12 +572,7 @@ private boolean retryBasedOnFailedReuseOfCachedHandle(SQLException e, // ANSI_NULLS and QUOTED_IDENTIFIER set options are not changed since the handle is prepared. // 8179: Could not find prepared statement with handle %d. // 99586: Error used for testing. - if (needsPrepare) { - return false; - } - else { - return 1 == attempt && (586 == e.getErrorCode() || 8179 == e.getErrorCode() || 99586 == e.getErrorCode()) && connection.isStatementPoolingEnabled(); - } + return 1 == attempt && (586 == e.getErrorCode() || 8179 == e.getErrorCode() || 99586 == e.getErrorCode()) && connection.isStatementPoolingEnabled(); } /** From 9ae0843e6eca96ed2fa5064a28764cdcba83e13c Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Wed, 14 Feb 2018 13:29:33 -0800 Subject: [PATCH 12/12] Remove unwanted method parameter --- .../sqlserver/jdbc/SQLServerPreparedStatement.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java index b133cc67a..0ec3d4aab 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java @@ -548,7 +548,7 @@ final void doExecutePreparedStatement(PrepStmtExecCmd command) throws SQLServerE getNextResult(); } catch (SQLException e) { - if (retryBasedOnFailedReuseOfCachedHandle(e, attempt, needsPrepare)) + if (retryBasedOnFailedReuseOfCachedHandle(e, attempt)) continue; else throw e; @@ -566,7 +566,7 @@ else if (EXECUTE_UPDATE == executeMethod && null != resultSet) { /** Should the execution be retried because the re-used cached handle could not be re-used due to server side state changes? */ private boolean retryBasedOnFailedReuseOfCachedHandle(SQLException e, - int attempt, boolean needsPrepare) { + int attempt) { // Only retry based on these error codes and if statementPooling is enabled: // 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. @@ -2692,7 +2692,7 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th throw e; // Retry if invalid handle exception. - if (retryBasedOnFailedReuseOfCachedHandle(e, attempt, needsPrepare)) { + if (retryBasedOnFailedReuseOfCachedHandle(e, attempt)) { // reset number of batches prepare numBatchesPrepared = numBatchesExecuted; retry = true; @@ -2723,7 +2723,7 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th } } catch (SQLException e) { - if (retryBasedOnFailedReuseOfCachedHandle(e, attempt, needsPrepare) && connection.isStatementPoolingEnabled()) { + if (retryBasedOnFailedReuseOfCachedHandle(e, attempt) && connection.isStatementPoolingEnabled()) { // Reset number of batches prepared. numBatchesPrepared = numBatchesExecuted; continue;