diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs index ac8c6d6d7d..e1367e5278 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -1176,6 +1176,11 @@ private object CompleteExecuteScalar(SqlDataReader ds, bool returnLastResult) } } } while (returnLastResult && ds.NextResult()); + + // Drain remaining results to ensure all error tokens are processed + // before returning the result (fix for GH issue #3736). + while (ds.NextResult()) + { } } finally { @@ -2853,7 +2858,7 @@ private Task InternalExecuteScalarAsync(CancellationToken cancellationTo { SqlDataReader reader = executeTask.Result; reader.ReadAsync(cancellationToken) - .ContinueWith((Task readTask) => + .ContinueWith(async (Task readTask) => { try { @@ -2886,6 +2891,11 @@ private Task InternalExecuteScalarAsync(CancellationToken cancellationTo exception = e; } } + + // Drain remaining results to ensure all error tokens are processed + // before returning the result (fix for GH issue #3736). + while (await reader.NextResultAsync(cancellationToken).ConfigureAwait(false)) + { } } finally { diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs index 1ef3967f20..dab51eee4c 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -1391,6 +1391,11 @@ private object CompleteExecuteScalar(SqlDataReader ds, bool returnLastResult) } } } while (returnLastResult && ds.NextResult()); + + // Drain remaining results to ensure all error tokens are processed + // before returning the result (fix for GH issue #3736). + while (ds.NextResult()) + { } } finally { @@ -3071,7 +3076,7 @@ private Task InternalExecuteScalarAsync(CancellationToken cancellationTo else { SqlDataReader reader = executeTask.Result; - reader.ReadAsync(cancellationToken).ContinueWith((readTask) => + reader.ReadAsync(cancellationToken).ContinueWith(async (readTask) => { try { @@ -3103,6 +3108,11 @@ private Task InternalExecuteScalarAsync(CancellationToken cancellationTo exception = e; } } + + // Drain remaining results to ensure all error tokens are processed + // before returning the result (fix for GH issue #3736). + while (await reader.NextResultAsync(cancellationToken).ConfigureAwait(false)) + { } } finally { diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj b/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj index e5d02df4d9..f890509c58 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj @@ -198,6 +198,7 @@ + diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/ProviderAgnostic/MultipleResultsTest/MultipleResultsTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/ProviderAgnostic/MultipleResultsTest/MultipleResultsTest.cs index 199b93e15f..77828f8ada 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/ProviderAgnostic/MultipleResultsTest/MultipleResultsTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/ProviderAgnostic/MultipleResultsTest/MultipleResultsTest.cs @@ -78,20 +78,15 @@ public void ExecuteScalar() { using SqlConnection connection = new SqlConnection((new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString) { MultipleActiveResultSets = true }).ConnectionString); using SqlCommand command = connection.CreateCommand(); - ConcurrentQueue messages = new ConcurrentQueue(); - - connection.InfoMessage += (object sender, SqlInfoMessageEventArgs args) => - messages.Enqueue(args.Message); connection.Open(); command.CommandText = s_sqlStatement; - // ExecuteScalar will select the first result set and the info message preceding it, then stop. - command.ExecuteScalar(); - Assert.True(messages.TryDequeue(out string lastMessage)); - Assert.Empty(messages); - Assert.Equal(ResultSet1_Message, lastMessage); + // ExecuteScalar now drains all result sets to ensure errors are not silently ignored (GH #3736 fix). + // Since the SQL statement contains RAISERRORs after the first result set, an exception is thrown. + SqlException ex = Assert.Throws(() => command.ExecuteScalar()); + Assert.Contains("Error 1", ex.Message); } [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs new file mode 100644 index 0000000000..ae6a783d30 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs @@ -0,0 +1,289 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.Data.SqlClient.ManualTesting.Tests +{ + /// + /// Tests for ExecuteScalar to verify proper exception handling. + /// Regression tests for GitHub issue #3736: https://github.com/dotnet/SqlClient/issues/3736 + /// + public static class SqlCommandExecuteScalarTest + { + /// + /// ExecuteScalar should propagate conversion errors that occur after the first result. + /// + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static void ExecuteScalar_ShouldThrowOnConversionError() + { + string tableName = DataTestUtility.GetLongName("GH3736"); + + using SqlConnection connection = new(DataTestUtility.TCPConnectionString); + connection.Open(); + + try + { + // Arrange + // Insert valid VARCHAR values - '42-43' is a valid string but not a valid number + DataTestUtility.CreateTable(connection, tableName, "(Id INT IDENTITY(1,1) NOT NULL PRIMARY KEY CLUSTERED, Val VARCHAR(10) NOT NULL)"); + using (SqlCommand insertCmd = connection.CreateCommand()) + { + insertCmd.CommandText = + $"INSERT INTO {tableName} (Val) VALUES ('12345'); " + + $"INSERT INTO {tableName} (Val) VALUES ('42-43');"; + insertCmd.ExecuteNonQuery(); + } + + // Act + // The WHERE clause compares VARCHAR to INT (no quotes around 12345), causing SQL Server + // to convert each Val to INT. '12345' converts fine, but '42-43' fails with error 245. + using SqlCommand cmd = new($"SELECT Id FROM {tableName} WHERE Val = 12345", connection); + SqlException ex = Assert.Throws(() => cmd.ExecuteScalar()); + + // Assert + Assert.Equal(245, ex.Number); + Assert.Contains("Conversion failed", ex.Message); + } + finally + { + DataTestUtility.DropTable(connection, tableName); + } + } + + /// + /// ExecuteScalar should throw on conversion error so transaction can be properly rolled back. + /// + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static void ExecuteScalar_TransactionShouldRollbackOnError() + { + string sourceTable = DataTestUtility.GetLongName("GH3736_Src"); + string targetTable = DataTestUtility.GetLongName("GH3736_Tgt"); + + using SqlConnection connection = new(DataTestUtility.TCPConnectionString); + connection.Open(); + + try + { + // Arrange + // sourceTable.Val is VARCHAR - both '12345' and '42-43' are valid strings but not valid numbers + DataTestUtility.CreateTable(connection, sourceTable, "(Id INT IDENTITY(1,1) NOT NULL PRIMARY KEY CLUSTERED, Val VARCHAR(10) NOT NULL)"); + DataTestUtility.CreateTable(connection, targetTable, "(Id INT IDENTITY(1,1) NOT NULL PRIMARY KEY CLUSTERED, Val1 INT NOT NULL, Val2 INT NOT NULL)"); + using (SqlCommand insertCmd = connection.CreateCommand()) + { + insertCmd.CommandText = + $"INSERT INTO {sourceTable} (Val) VALUES ('12345'); " + + $"INSERT INTO {sourceTable} (Val) VALUES ('42-43');"; + insertCmd.ExecuteNonQuery(); + } + + // Act + // The conversion error occurs in cmd2's WHERE clause (Val = 12345 without quotes), + // not during the INSERT statements above. + SqlException ex = Assert.Throws(() => + { + using SqlTransaction transaction = connection.BeginTransaction(); + using (SqlCommand cmd1 = new($"INSERT INTO {targetTable} (Val1, Val2) VALUES (42, 43)", connection, transaction)) + { + cmd1.ExecuteNonQuery(); + } + using (SqlCommand cmd2 = new($"SELECT Id FROM {sourceTable} WHERE Val = 12345", connection, transaction)) + { + cmd2.ExecuteScalar(); + } + using (SqlCommand cmd3 = new($"INSERT INTO {targetTable} (Val1, Val2) VALUES (100, 200)", connection, transaction)) + { + cmd3.ExecuteNonQuery(); + } + transaction.Commit(); + }); + + // Assert + Assert.Equal(245, ex.Number); + using (SqlCommand verifyCmd = new($"SELECT COUNT(*) FROM {targetTable}", connection)) + { + int count = (int)verifyCmd.ExecuteScalar(); + Assert.Equal(0, count); + } + } + finally + { + DataTestUtility.DropTable(connection, sourceTable); + DataTestUtility.DropTable(connection, targetTable); + } + } + + /// + /// ExecuteScalar should work correctly when there is no error. + /// + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static void ExecuteScalar_ShouldWorkCorrectlyWithoutError() + { + // Arrange + using SqlConnection connection = new(DataTestUtility.TCPConnectionString); + connection.Open(); + using SqlCommand cmd = new("SELECT 42", connection); + + // Act + object result = cmd.ExecuteScalar(); + + // Assert + Assert.Equal(42, result); + } + + /// + /// ExecuteScalar should return null when there are no rows. + /// + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static void ExecuteScalar_ShouldReturnNullWhenNoRows() + { + // Arrange + using SqlConnection connection = new(DataTestUtility.TCPConnectionString); + connection.Open(); + using SqlCommand cmd = new("SELECT 1 WHERE 1 = 0", connection); + + // Act + object result = cmd.ExecuteScalar(); + + // Assert + Assert.Null(result); + } + + /// + /// ExecuteScalarAsync should propagate conversion errors that occur after the first result. + /// + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static async Task ExecuteScalarAsync_ShouldThrowOnConversionError() + { + string tableName = DataTestUtility.GetLongName("GH3736_Async"); + + using SqlConnection connection = new(DataTestUtility.TCPConnectionString); + await connection.OpenAsync(); + + try + { + // Arrange + DataTestUtility.CreateTable(connection, tableName, "(Id INT IDENTITY(1,1) NOT NULL PRIMARY KEY CLUSTERED, Val VARCHAR(10) NOT NULL)"); + using (SqlCommand insertCmd = connection.CreateCommand()) + { + insertCmd.CommandText = + $"INSERT INTO {tableName} (Val) VALUES ('12345'); " + + $"INSERT INTO {tableName} (Val) VALUES ('42-43');"; + await insertCmd.ExecuteNonQueryAsync(); + } + + // Act + using SqlCommand cmd = new($"SELECT Id FROM {tableName} WHERE Val = 12345", connection); + SqlException ex = await Assert.ThrowsAsync(() => cmd.ExecuteScalarAsync()); + + // Assert + Assert.Equal(245, ex.Number); + Assert.Contains("Conversion failed", ex.Message); + } + finally + { + DataTestUtility.DropTable(connection, tableName); + } + } + + /// + /// ExecuteScalarAsync should throw on conversion error so transaction can be properly rolled back. + /// + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static async Task ExecuteScalarAsync_TransactionShouldRollbackOnError() + { + string sourceTable = DataTestUtility.GetLongName("GH3736_AsyncSrc"); + string targetTable = DataTestUtility.GetLongName("GH3736_AsyncTgt"); + + using SqlConnection connection = new(DataTestUtility.TCPConnectionString); + await connection.OpenAsync(); + + try + { + // Arrange + // sourceTable.Val is VARCHAR - both '12345' and '42-43' are valid strings but not valid numbers + DataTestUtility.CreateTable(connection, sourceTable, "(Id INT IDENTITY(1,1) NOT NULL PRIMARY KEY CLUSTERED, Val VARCHAR(10) NOT NULL)"); + DataTestUtility.CreateTable(connection, targetTable, "(Id INT IDENTITY(1,1) NOT NULL PRIMARY KEY CLUSTERED, Val1 INT NOT NULL, Val2 INT NOT NULL)"); + using (SqlCommand insertCmd = connection.CreateCommand()) + { + insertCmd.CommandText = + $"INSERT INTO {sourceTable} (Val) VALUES ('12345'); " + + $"INSERT INTO {sourceTable} (Val) VALUES ('42-43');"; + await insertCmd.ExecuteNonQueryAsync(); + } + + // Act + // The conversion error occurs in cmd2's WHERE clause (Val = 12345 without quotes), + // not during the INSERT statements above. + SqlException ex = await Assert.ThrowsAsync(async () => + { + using SqlTransaction transaction = connection.BeginTransaction(); + using (SqlCommand cmd1 = new($"INSERT INTO {targetTable} (Val1, Val2) VALUES (42, 43)", connection, transaction)) + { + await cmd1.ExecuteNonQueryAsync(); + } + using (SqlCommand cmd2 = new($"SELECT Id FROM {sourceTable} WHERE Val = 12345", connection, transaction)) + { + await cmd2.ExecuteScalarAsync(); + } + using (SqlCommand cmd3 = new($"INSERT INTO {targetTable} (Val1, Val2) VALUES (100, 200)", connection, transaction)) + { + await cmd3.ExecuteNonQueryAsync(); + } + transaction.Commit(); + }); + + // Assert + Assert.Equal(245, ex.Number); + using (SqlCommand verifyCmd = new($"SELECT COUNT(*) FROM {targetTable}", connection)) + { + int count = (int)await verifyCmd.ExecuteScalarAsync(); + Assert.Equal(0, count); + } + } + finally + { + DataTestUtility.DropTable(connection, sourceTable); + DataTestUtility.DropTable(connection, targetTable); + } + } + + /// + /// ExecuteScalarAsync should work correctly when there is no error. + /// + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static async Task ExecuteScalarAsync_ShouldWorkCorrectlyWithoutError() + { + // Arrange + using SqlConnection connection = new(DataTestUtility.TCPConnectionString); + await connection.OpenAsync(); + using SqlCommand cmd = new("SELECT 42", connection); + + // Act + object result = await cmd.ExecuteScalarAsync(); + + // Assert + Assert.Equal(42, result); + } + + /// + /// ExecuteScalarAsync should return null when there are no rows. + /// + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static async Task ExecuteScalarAsync_ShouldReturnNullWhenNoRows() + { + // Arrange + using SqlConnection connection = new(DataTestUtility.TCPConnectionString); + await connection.OpenAsync(); + using SqlCommand cmd = new("SELECT 1 WHERE 1 = 0", connection); + + // Act + object result = await cmd.ExecuteScalarAsync(); + + // Assert + Assert.Null(result); + } + } +}