diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index a8397b605..7caf1b172 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -83,6 +83,9 @@ // 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; @@ -3079,6 +3082,8 @@ 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"); @@ -5760,6 +5765,12 @@ 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 b180a11c9..3117ec403 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionPoolProxy.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionPoolProxy.java @@ -168,6 +168,9 @@ public void run() { if (bIsOpen && (null != wrappedConnection)) { 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 2437a82d0..d18ebffb7 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java @@ -51,6 +51,8 @@ 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; @@ -326,6 +328,13 @@ private boolean buildPreparedStrings(Parameter[] params, String newTypeDefinitions = buildParamTypeDefinitions(params, renewDefinition); if (null != preparedTypeDefinitions && newTypeDefinitions.equals(preparedTypeDefinitions)) return false; + + if(preparedTypeDefinitions == null) { + definitionChanged = false; + } + else { + definitionChanged = true; + } preparedTypeDefinitions = newTypeDefinitions; @@ -486,6 +495,8 @@ 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. @@ -524,33 +535,20 @@ final void doExecutePreparedStatement(PrepStmtExecCmd command) throws SQLServerE hasNewTypeDefinitions = buildPreparedStrings(inOutParam, true); } - // 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)) { - 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); - - doPrepExec(tdsWriter, inOutParam, hasNewTypeDefinitions, hasExistingTypeDefinitions); - - ensureExecuteResultsReader(command.startResponse(getIsResponseBufferingAdaptive())); - startResults(); - getNextResult(); - } - catch(SQLException e) { - if (retryBasedOnFailedReuseOfCachedHandle(e, attempt)) - continue; - else - throw e; - } - break; + if (reuseCachedHandle(hasNewTypeDefinitions, false)) { + 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); + + doPrepExec(tdsWriter, inOutParam, hasNewTypeDefinitions, hasExistingTypeDefinitions); + + ensureExecuteResultsReader(command.startResponse(getIsResponseBufferingAdaptive())); + startResults(); + getNextResult(); + if (EXECUTE_QUERY == executeMethod && null == resultSet) { SQLServerException.makeFromDriverError(connection, this, SQLServerException.getErrString("R_noResultset"), null, true); } @@ -559,15 +557,6 @@ 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) { - // 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. * @@ -916,7 +905,18 @@ private void getParameterEncryptionMetadata(Parameter[] params) throws SQLServer /** Manage re-using cached handles */ private boolean reuseCachedHandle(boolean hasNewTypeDefinitions, boolean discardCurrentCacheItem) { - + 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; @@ -2565,146 +2565,124 @@ 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]; - TDSWriter tdsWriter = null; - 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); + definitionChanged = 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); + 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); + } + } - // 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); + // 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); + } } - } - // 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 (reuseCachedHandle(hasNewTypeDefinitions, false)) { + 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)) { - hasNewTypeDefinitions = false; - } - - if (numBatchesExecuted < numBatchesPrepared) { - // assert null != tdsWriter; - tdsWriter.writeByte((byte) nBatchStatementDelimiter); - } - else { - resetForReexecute(); - tdsWriter = batchCommand.startRequest(TDS.PKT_RPC); - } + if (numBatchesExecuted < numBatchesPrepared) { + // assert null != tdsWriter; + tdsWriter.writeByte((byte) nBatchStatementDelimiter); + } + else { + resetForReexecute(); + tdsWriter = batchCommand.startRequest(TDS.PKT_RPC); + } - // 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; + // 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); } - - // 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; + 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; + } - // Only way to proceed with preparing the next set of batches is if - // we successfully executed the previously prepared set. - assert numBatchesExecuted == numBatchesPrepared; - } - } - 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; + // 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++; } + + // Only way to proceed with preparing the next set of batches is if + // we successfully executed the previously prepared set. + assert numBatchesExecuted == numBatchesPrepared; } - break; + } + } + catch (SQLServerException e) { + // throw the initial batchException + if (null != batchCommand.batchException) { + throw batchCommand.batchException; + } + else { + throw e; } } } diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java index 7f6d8e1d2..9385fd165 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java @@ -800,6 +800,21 @@ 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(); @@ -811,6 +826,8 @@ 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. @@ -914,7 +931,9 @@ private void doExecuteStatementBatch(StmtBatchExecCmd execCmd) throws SQLServerE tdsWriter.writeString(batchIter.next()); while (batchIter.hasNext()) { tdsWriter.writeString(" ; "); - tdsWriter.writeString(batchIter.next()); + 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 944c79672..b25025561 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,12 +8,13 @@ package com.microsoft.sqlserver.jdbc.unit.statement; import static java.util.concurrent.TimeUnit.SECONDS; -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.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; 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 { @@ -172,32 +173,69 @@ 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) 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 = 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; + } + } - int[] updateCounts = pstmt.executeBatch(); + // since executeBatch() throws exception, it does not return anthing. So updateCounts is still null. + assertSame(null, updateCounts); - // Verify update counts are correct - for (int i : updateCounts) { - assertSame(1, i); + // 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)); } } }