From ee91dbc5a06d11c74832abe9ffdb41769a804972 Mon Sep 17 00:00:00 2001 From: Saransh Sharma Date: Fri, 23 Jan 2026 13:02:42 -0800 Subject: [PATCH 1/5] Fix github issue 3736 --- .../Data/SqlClient/SqlCommand.Scalar.cs | 8 + ....Data.SqlClient.ManualTesting.Tests.csproj | 1 + .../SqlCommand/SqlCommandExecuteScalarTest.cs | 183 ++++++++++++++++++ 3 files changed, 192 insertions(+) create mode 100644 src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs 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..3338d2200a 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,14 @@ 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). + if (!returnLastResult) + { + while (reader.NextResult()) + { } + } } 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/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs new file mode 100644 index 0000000000..9765ad6ec3 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs @@ -0,0 +1,183 @@ +// 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; +using Xunit; + +namespace Microsoft.Data.SqlClient.ManualTesting.Tests +{ + /// + /// Tests for ExecuteScalar to verify proper exception handling. + /// + public static class SqlCommandExecuteScalarTest + { + + private static string GenerateTableName(string prefix) => + $"##{prefix}_{Guid.NewGuid():N}"; + + /// + /// Regression test for GitHub issue #3736: https://github.com/dotnet/SqlClient/issues/3736 + /// ExecuteScalar should properly propagate conversion errors that occur after the first result. + /// + /// Without the fix, the conversion error is swallowed during reader.Close(), causing: + /// 1. The transaction to become "zombied" without throwing an exception + /// 2. Subsequent commands to execute on a dead transaction + /// 3. Partial commits when the transaction commit fails + /// + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static void ExecuteScalar_ShouldThrowOnConversionError_GH3736() + { + string tab1Name = GenerateTableName("tab1"); + string tab2Name = GenerateTableName("tab2"); + + using SqlConnection connection = new(DataTestUtility.TCPConnectionString); + connection.Open(); + + // Setup: Create test tables (temp tables auto-cleanup) + using (SqlCommand setupCmd = connection.CreateCommand()) + { + setupCmd.CommandText = $@" + CREATE TABLE {tab1Name} ( + [Id] INT IDENTITY(1,1) NOT NULL, + [Val] VARCHAR(10) NOT NULL + ); + + CREATE TABLE {tab2Name} ( + [Id] INT IDENTITY(1,1) NOT NULL, + [Val1] INT NOT NULL, + [Val2] INT NOT NULL + ); + + INSERT INTO {tab1Name} (Val) VALUES ('12345'); + INSERT INTO {tab1Name} (Val) VALUES ('42-43');"; // This will cause conversion error + setupCmd.ExecuteNonQuery(); + } + + // Test: Execute a query that will cause a conversion error after returning a valid row + // The query "SELECT Id FROM tab1 WHERE Val = 12345" will: + // 1. Return row with Id=1 (Val='12345' converts to 12345) + // 2. Fail on row with Id=2 (Val='42-43' cannot convert to int) + // + // Before the fix: ExecuteScalar returns 1 without throwing an exception + // After the fix: ExecuteScalar throws SqlException with the conversion error + using SqlCommand cmd = new($"SELECT Id FROM {tab1Name} WHERE Val = 12345", connection); + + SqlException ex = Assert.Throws(() => cmd.ExecuteScalar()); + + // Error 245: Conversion failed when converting the varchar value '42-43' to data type int. + Assert.Equal(245, ex.Number); + Assert.Contains("Conversion failed", ex.Message); + } + + /// + /// Regression test for GitHub issue #3736: + /// Verifies the transaction scenario from the original issue report. + /// + /// In the original bug, a transaction would be zombied without notification, causing: + /// - INSERT before the error to be rolled back (correct) + /// - INSERT after the error to be committed outside transaction (incorrect) + /// + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static void ExecuteScalar_TransactionShouldRollbackOnError_GH3736() + { + string tab1Name = GenerateTableName("tab1"); + string tab2Name = GenerateTableName("tab2"); + + using SqlConnection connection = new(DataTestUtility.TCPConnectionString); + connection.Open(); + + // Setup: Create test tables (temp tables auto-cleanup) + using (SqlCommand setupCmd = connection.CreateCommand()) + { + setupCmd.CommandText = $@" + CREATE TABLE {tab1Name} ( + [Id] INT IDENTITY(1,1) NOT NULL, + [Val] VARCHAR(10) NOT NULL + ); + + CREATE TABLE {tab2Name} ( + [Id] INT IDENTITY(1,1) NOT NULL, + [Val1] INT NOT NULL, + [Val2] INT NOT NULL + ); + + INSERT INTO {tab1Name} (Val) VALUES ('12345'); + INSERT INTO {tab1Name} (Val) VALUES ('42-43');"; + setupCmd.ExecuteNonQuery(); + } + + // Test: Execute queries in a transaction where one will cause an error + bool exceptionThrown = false; + try + { + using SqlTransaction transaction = connection.BeginTransaction(); + + // First insert - should be rolled back if transaction fails + using (SqlCommand cmd1 = new($"INSERT INTO {tab2Name} (Val1, Val2) VALUES (42, 43)", connection, transaction)) + { + cmd1.ExecuteNonQuery(); + } + + // This should throw due to conversion error + using (SqlCommand cmd2 = new($"SELECT Id FROM {tab1Name} WHERE Val = 12345", connection, transaction)) + { + cmd2.ExecuteScalar(); + } + + // This should never execute if the fix is working + using (SqlCommand cmd3 = new($"INSERT INTO {tab2Name} (Val1, Val2) VALUES (100, 200)", connection, transaction)) + { + cmd3.ExecuteNonQuery(); + } + + transaction.Commit(); + } + catch (SqlException ex) when (ex.Number == 245) + { + // Expected: Conversion error should be thrown + exceptionThrown = true; + } + + Assert.True(exceptionThrown, "Expected SqlException with conversion error (245) was not thrown"); + + // Verify: No rows should be in tab2 (transaction should have been rolled back) + using (SqlCommand verifyCmd = new($"SELECT COUNT(*) FROM {tab2Name}", connection)) + { + int count = (int)verifyCmd.ExecuteScalar(); + Assert.Equal(0, count); + } + } + + /// + /// Verifies that ExecuteScalar works correctly when there is no error. + /// This is a sanity check to ensure the fix doesn't break normal functionality. + /// + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static void ExecuteScalar_ShouldWorkCorrectlyWithoutError() + { + using SqlConnection connection = new(DataTestUtility.TCPConnectionString); + connection.Open(); + + using SqlCommand cmd = new("SELECT 42", connection); + object result = cmd.ExecuteScalar(); + + Assert.Equal(42, result); + } + + /// + /// Verifies that ExecuteScalar returns null when there are no rows. + /// + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static void ExecuteScalar_ShouldReturnNullWhenNoRows() + { + using SqlConnection connection = new(DataTestUtility.TCPConnectionString); + connection.Open(); + + using SqlCommand cmd = new("SELECT 1 WHERE 1 = 0", connection); + object result = cmd.ExecuteScalar(); + + Assert.Null(result); + } + } +} From da9bc620233f9e523fef7b966e6c0693993027b8 Mon Sep 17 00:00:00 2001 From: Saransh Sharma Date: Wed, 28 Jan 2026 13:19:10 -0800 Subject: [PATCH 2/5] Add async path fix and tests --- .../Data/SqlClient/SqlCommand.Scalar.cs | 12 +- .../SqlCommand/SqlCommandExecuteScalarTest.cs | 329 ++++++++++++------ 2 files changed, 228 insertions(+), 113 deletions(-) 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 3338d2200a..b9ee6d4ada 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 @@ -187,11 +187,8 @@ private static object CompleteExecuteScalar(SqlDataReader reader, bool returnLas // Drain remaining results to ensure all error tokens are processed // before returning the result (fix for GH issue #3736). - if (!returnLastResult) - { - while (reader.NextResult()) - { } - } + while (reader.NextResult()) + { } } finally { @@ -306,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 (reader.NextResult()) + { } } finally { diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs index 9765ad6ec3..71a36e6439 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs @@ -2,181 +2,294 @@ // 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; +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 { - - private static string GenerateTableName(string prefix) => - $"##{prefix}_{Guid.NewGuid():N}"; - /// - /// Regression test for GitHub issue #3736: https://github.com/dotnet/SqlClient/issues/3736 - /// ExecuteScalar should properly propagate conversion errors that occur after the first result. - /// - /// Without the fix, the conversion error is swallowed during reader.Close(), causing: - /// 1. The transaction to become "zombied" without throwing an exception - /// 2. Subsequent commands to execute on a dead transaction - /// 3. Partial commits when the transaction commit fails + /// ExecuteScalar should propagate conversion errors that occur after the first result. /// [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] - public static void ExecuteScalar_ShouldThrowOnConversionError_GH3736() + public static void ExecuteScalar_ShouldThrowOnConversionError() { - string tab1Name = GenerateTableName("tab1"); - string tab2Name = GenerateTableName("tab2"); + string tableName = DataTestUtility.GetLongName("GH3736"); using SqlConnection connection = new(DataTestUtility.TCPConnectionString); connection.Open(); - // Setup: Create test tables (temp tables auto-cleanup) - using (SqlCommand setupCmd = connection.CreateCommand()) + try { - setupCmd.CommandText = $@" - CREATE TABLE {tab1Name} ( - [Id] INT IDENTITY(1,1) NOT NULL, - [Val] VARCHAR(10) NOT NULL - ); - - CREATE TABLE {tab2Name} ( - [Id] INT IDENTITY(1,1) NOT NULL, - [Val1] INT NOT NULL, - [Val2] INT NOT NULL - ); - - INSERT INTO {tab1Name} (Val) VALUES ('12345'); - INSERT INTO {tab1Name} (Val) VALUES ('42-43');"; // This will cause conversion error - setupCmd.ExecuteNonQuery(); - } + // 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(); + } - // Test: Execute a query that will cause a conversion error after returning a valid row - // The query "SELECT Id FROM tab1 WHERE Val = 12345" will: - // 1. Return row with Id=1 (Val='12345' converts to 12345) - // 2. Fail on row with Id=2 (Val='42-43' cannot convert to int) - // - // Before the fix: ExecuteScalar returns 1 without throwing an exception - // After the fix: ExecuteScalar throws SqlException with the conversion error - using SqlCommand cmd = new($"SELECT Id FROM {tab1Name} WHERE Val = 12345", connection); - - SqlException ex = Assert.Throws(() => cmd.ExecuteScalar()); - - // Error 245: Conversion failed when converting the varchar value '42-43' to data type int. - Assert.Equal(245, ex.Number); - Assert.Contains("Conversion failed", ex.Message); + // 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); + } } /// - /// Regression test for GitHub issue #3736: - /// Verifies the transaction scenario from the original issue report. - /// - /// In the original bug, a transaction would be zombied without notification, causing: - /// - INSERT before the error to be rolled back (correct) - /// - INSERT after the error to be committed outside transaction (incorrect) + /// ExecuteScalar should throw on conversion error so transaction can be properly rolled back. /// [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] - public static void ExecuteScalar_TransactionShouldRollbackOnError_GH3736() + public static void ExecuteScalar_TransactionShouldRollbackOnError() { - string tab1Name = GenerateTableName("tab1"); - string tab2Name = GenerateTableName("tab2"); + string sourceTable = DataTestUtility.GetLongName("GH3736_Src"); + string targetTable = DataTestUtility.GetLongName("GH3736_Tgt"); using SqlConnection connection = new(DataTestUtility.TCPConnectionString); connection.Open(); - // Setup: Create test tables (temp tables auto-cleanup) - using (SqlCommand setupCmd = connection.CreateCommand()) - { - setupCmd.CommandText = $@" - CREATE TABLE {tab1Name} ( - [Id] INT IDENTITY(1,1) NOT NULL, - [Val] VARCHAR(10) NOT NULL - ); - - CREATE TABLE {tab2Name} ( - [Id] INT IDENTITY(1,1) NOT NULL, - [Val1] INT NOT NULL, - [Val2] INT NOT NULL - ); - - INSERT INTO {tab1Name} (Val) VALUES ('12345'); - INSERT INTO {tab1Name} (Val) VALUES ('42-43');"; - setupCmd.ExecuteNonQuery(); - } - - // Test: Execute queries in a transaction where one will cause an error - bool exceptionThrown = false; try { - using SqlTransaction transaction = connection.BeginTransaction(); - - // First insert - should be rolled back if transaction fails - using (SqlCommand cmd1 = new($"INSERT INTO {tab2Name} (Val1, Val2) VALUES (42, 43)", connection, transaction)) + // 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()) { - cmd1.ExecuteNonQuery(); + insertCmd.CommandText = + $"INSERT INTO {sourceTable} (Val) VALUES ('12345'); " + + $"INSERT INTO {sourceTable} (Val) VALUES ('42-43');"; + insertCmd.ExecuteNonQuery(); } - // This should throw due to conversion error - using (SqlCommand cmd2 = new($"SELECT Id FROM {tab1Name} WHERE Val = 12345", connection, transaction)) + // Act + // The conversion error occurs in cmd2's WHERE clause (Val = 12345 without quotes), + // not during the INSERT statements above. + bool exceptionThrown = false; + try { - cmd2.ExecuteScalar(); + 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(); } - - // This should never execute if the fix is working - using (SqlCommand cmd3 = new($"INSERT INTO {tab2Name} (Val1, Val2) VALUES (100, 200)", connection, transaction)) + catch (SqlException ex) when (ex.Number == 245) { - cmd3.ExecuteNonQuery(); + exceptionThrown = true; } - transaction.Commit(); - } - catch (SqlException ex) when (ex.Number == 245) - { - // Expected: Conversion error should be thrown - exceptionThrown = true; + // Assert + Assert.True(exceptionThrown, "Expected SqlException with conversion error (245) was not thrown"); + using (SqlCommand verifyCmd = new($"SELECT COUNT(*) FROM {targetTable}", connection)) + { + int count = (int)verifyCmd.ExecuteScalar(); + Assert.Equal(0, count); + } } - - Assert.True(exceptionThrown, "Expected SqlException with conversion error (245) was not thrown"); - - // Verify: No rows should be in tab2 (transaction should have been rolled back) - using (SqlCommand verifyCmd = new($"SELECT COUNT(*) FROM {tab2Name}", connection)) + finally { - int count = (int)verifyCmd.ExecuteScalar(); - Assert.Equal(0, count); + DataTestUtility.DropTable(connection, sourceTable); + DataTestUtility.DropTable(connection, targetTable); } } /// - /// Verifies that ExecuteScalar works correctly when there is no error. - /// This is a sanity check to ensure the fix doesn't break normal functionality. + /// 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); } /// - /// Verifies that ExecuteScalar returns null when there are no rows. + /// 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 + 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 + bool exceptionThrown = false; + try + { + 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(); + } + catch (SqlException ex) when (ex.Number == 245) + { + exceptionThrown = true; + } + + // Assert + Assert.True(exceptionThrown, "Expected SqlException with conversion error (245) was not thrown"); + 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); } } From 86b33ba2bd17fd3e01bb70bdffc91b7ab57eda39 Mon Sep 17 00:00:00 2001 From: Saransh Sharma Date: Wed, 28 Jan 2026 14:40:27 -0800 Subject: [PATCH 3/5] Consistent exception handling in tests --- .../SqlCommand/SqlCommandExecuteScalarTest.cs | 22 +++++-------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs index 71a36e6439..c1025334d3 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs @@ -82,8 +82,7 @@ public static void ExecuteScalar_TransactionShouldRollbackOnError() // Act // The conversion error occurs in cmd2's WHERE clause (Val = 12345 without quotes), // not during the INSERT statements above. - bool exceptionThrown = false; - try + SqlException ex = Assert.Throws(() => { using SqlTransaction transaction = connection.BeginTransaction(); using (SqlCommand cmd1 = new($"INSERT INTO {targetTable} (Val1, Val2) VALUES (42, 43)", connection, transaction)) @@ -99,14 +98,10 @@ public static void ExecuteScalar_TransactionShouldRollbackOnError() cmd3.ExecuteNonQuery(); } transaction.Commit(); - } - catch (SqlException ex) when (ex.Number == 245) - { - exceptionThrown = true; - } + }); // Assert - Assert.True(exceptionThrown, "Expected SqlException with conversion error (245) was not thrown"); + Assert.Equal(245, ex.Number); using (SqlCommand verifyCmd = new($"SELECT COUNT(*) FROM {targetTable}", connection)) { int count = (int)verifyCmd.ExecuteScalar(); @@ -219,8 +214,7 @@ public static async Task ExecuteScalarAsync_TransactionShouldRollbackOnError() } // Act - bool exceptionThrown = false; - try + 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)) @@ -236,14 +230,10 @@ public static async Task ExecuteScalarAsync_TransactionShouldRollbackOnError() await cmd3.ExecuteNonQueryAsync(); } transaction.Commit(); - } - catch (SqlException ex) when (ex.Number == 245) - { - exceptionThrown = true; - } + }); // Assert - Assert.True(exceptionThrown, "Expected SqlException with conversion error (245) was not thrown"); + Assert.Equal(245, ex.Number); using (SqlCommand verifyCmd = new($"SELECT COUNT(*) FROM {targetTable}", connection)) { int count = (int)await verifyCmd.ExecuteScalarAsync(); From 83805940b1c85d2eb50cc62e4b552f9fece21220 Mon Sep 17 00:00:00 2001 From: Saransh Sharma Date: Thu, 29 Jan 2026 14:58:02 -0800 Subject: [PATCH 4/5] Update MultipleResultTests --- .../MultipleResultsTest/MultipleResultsTest.cs | 9 ++++----- .../SQL/SqlCommand/SqlCommandExecuteScalarTest.cs | 3 +++ 2 files changed, 7 insertions(+), 5 deletions(-) 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 index c1025334d3..590657e5db 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs @@ -203,6 +203,7 @@ public static async Task ExecuteScalarAsync_TransactionShouldRollbackOnError() 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()) @@ -214,6 +215,8 @@ public static async Task ExecuteScalarAsync_TransactionShouldRollbackOnError() } // 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(); From 47fb5e5ab7ed9f8d59f5686e6632b24aa70ebaf8 Mon Sep 17 00:00:00 2001 From: Saransh Sharma Date: Thu, 29 Jan 2026 15:04:58 -0800 Subject: [PATCH 5/5] Fix async path --- .../src/Microsoft/Data/SqlClient/SqlCommand.Scalar.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 b9ee6d4ada..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 @@ -261,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 @@ -306,7 +306,7 @@ private Task ExecuteScalarAsyncInternal(CancellationToken cancellationTo // Drain remaining results to ensure all error tokens are processed // before returning the result (fix for GH issue #3736). - while (reader.NextResult()) + while (await reader.NextResultAsync(cancellationToken).ConfigureAwait(false)) { } } finally