diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Scalar.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Scalar.cs index f44e4476d5..64c12c4ecb 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Scalar.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Scalar.cs @@ -184,6 +184,11 @@ private static object CompleteExecuteScalar(SqlDataReader reader, bool returnLas result = reader.GetValue(0); } } while (returnLastResult && reader.NextResult()); + + // Drain remaining results to ensure all error tokens are processed + // before returning the result (fix for GH issue #3736). + while (reader.NextResult()) + { } } finally { @@ -256,7 +261,7 @@ private Task ExecuteScalarAsyncInternal(CancellationToken cancellationTo SqlDataReader reader = executeTask.Result; // @TODO: Use continue with state? - reader.ReadAsync(cancellationToken).ContinueWith(readTask => + reader.ReadAsync(cancellationToken).ContinueWith(async readTask => { // @TODO: This seems a bit confusing with unnecessary extra dispose calls and try/finally blocks try @@ -298,6 +303,11 @@ private Task ExecuteScalarAsyncInternal(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 3155b87f81..01133f2ec2 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 @@ -188,6 +188,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..bf1551373b 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/ProviderAgnostic/MultipleResultsTest/MultipleResultsTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/ProviderAgnostic/MultipleResultsTest/MultipleResultsTest.cs @@ -87,11 +87,10 @@ public void ExecuteScalar() 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..590657e5db --- /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, not an invalid number + DataTestUtility.CreateTable(connection, tableName, "(Id INT IDENTITY(1,1) NOT NULL, 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 + DataTestUtility.CreateTable(connection, sourceTable, "(Id INT IDENTITY(1,1) NOT NULL, Val VARCHAR(10) NOT NULL)"); + DataTestUtility.CreateTable(connection, targetTable, "(Id INT IDENTITY(1,1) NOT NULL, 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, 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 + DataTestUtility.CreateTable(connection, sourceTable, "(Id INT IDENTITY(1,1) NOT NULL, Val VARCHAR(10) NOT NULL)"); + DataTestUtility.CreateTable(connection, targetTable, "(Id INT IDENTITY(1,1) NOT NULL, 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); + } + } +}