Skip to content

[6.1] Port #3912 to release/6.1 #3947

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
samsharma2700 wants to merge 1 commit into
base: release/6.1
Choose a base branch
from
Open

[6.1] Port #3912 to release/6.1 #3947

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -2853,7 +2858,7 @@ private Task<object> InternalExecuteScalarAsync(CancellationToken cancellationTo
{
SqlDataReader reader = executeTask.Result;
reader.ReadAsync(cancellationToken)
.ContinueWith((Task<bool> readTask) =>
.ContinueWith(async (Task<bool> readTask) =>
{
try
{
Expand Down Expand Up @@ -2886,6 +2891,11 @@ private Task<object> 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))
{ }
Comment on lines +2894 to +2898
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this async path, draining remaining result sets uses the user-provided cancellationToken. If the token is canceled after the first ReadAsync completes, NextResultAsync will typically throw OperationCanceledException which is currently caught by the generic catch and reported via source.SetException(e), producing a faulted task rather than a canceled task. Consider handling OperationCanceledException/TaskCanceledException explicitly (or checking cancellationToken.IsCancellationRequested) and calling source.SetCanceled() to preserve ExecuteScalarAsync cancellation semantics.

Copilot uses AI. Check for mistakes.
}
finally
{
Expand Down
12 changes: 11 additions & 1 deletion src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -3071,7 +3076,7 @@ private Task<object> InternalExecuteScalarAsync(CancellationToken cancellationTo
else
{
SqlDataReader reader = executeTask.Result;
reader.ReadAsync(cancellationToken).ContinueWith((readTask) =>
reader.ReadAsync(cancellationToken).ContinueWith(async (readTask) =>
{
try
{
Expand Down Expand Up @@ -3103,6 +3108,11 @@ private Task<object> 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))
{ }
Comment on lines +3112 to +3115
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this async path, the added drain loop awaits NextResultAsync(cancellationToken). If cancellation is requested after the initial ReadAsync has completed, NextResultAsync can throw OperationCanceledException which is currently handled by the generic catch and surfaced via source.SetException(e), yielding a faulted task instead of a canceled task. Consider catching OperationCanceledException/TaskCanceledException (or checking cancellationToken.IsCancellationRequested) and calling source.SetCanceled() to keep ExecuteScalarAsync cancellation behavior consistent.

Copilot uses AI. Check for mistakes.
}
finally
{
Expand Down
1 change: 1 addition & 0 deletions ...soft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@
<Compile Include="SQL\SplitPacketTest\SplitPacketTest.cs" />
<Compile Include="SQL\SqlCommand\SqlCommandCompletedTest.cs" />
<Compile Include="SQL\SqlCommand\SqlCommandCancelTest.cs" />
<Compile Include="SQL\SqlCommand\SqlCommandExecuteScalarTest.cs" />
<Compile Include="SQL\SqlCommand\SqlCommandSetTest.cs" />
<Compile Include="SQL\SqlCredentialTest\SqlCredentialTest.cs" />
<Compile Include="SQL\SqlDependencyTest\SqlDependencyTest.cs" />
Expand Down
9 changes: 4 additions & 5 deletions ...a.SqlClient/tests/ManualTests/ProviderAgnostic/MultipleResultsTest/MultipleResultsTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<SqlException>(() => command.ExecuteScalar());
Assert.Contains("Error 1", ex.Message);
}

[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
Expand Down
289 changes: 289 additions & 0 deletions src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
{
/// <summary>
/// Tests for ExecuteScalar to verify proper exception handling.
/// Regression tests for GitHub issue #3736: https://github.com/dotnet/SqlClient/issues/3736
/// </summary>
public static class SqlCommandExecuteScalarTest
{
/// <summary>
/// ExecuteScalar should propagate conversion errors that occur after the first result.
/// </summary>
[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
Copy link
Contributor

@paulmedynski paulmedynski Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... is a valid string, but not a valid number.

DataTestUtility.CreateTable(connection, tableName, "(Id INT IDENTITY(1,1) NOT NULL, Val VARCHAR(10) NOT NULL)");
using (SqlCommand insertCmd = connection.CreateCommand())
{
Comment on lines +29 to +33
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These regression tests rely on SQL Server returning at least one row before hitting the conversion error (data token followed by error token). With the current table definition (heap without a clustered key), scan order isn't guaranteed and the conversion error can occur before any row is returned, which may not exercise the specific failure mode this PR targets. Consider adding a clustered primary key (e.g., on Id) so the scan order is deterministic (Id 1 row returned before the Id 2 row triggers error 245).

Copilot generated this review using guidance from repository custom instructions.
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<SqlException>(() => cmd.ExecuteScalar());

// Assert
Assert.Equal(245, ex.Number);
Assert.Contains("Conversion failed", ex.Message);
}
finally
{
DataTestUtility.DropTable(connection, tableName);
}
}

/// <summary>
/// ExecuteScalar should throw on conversion error so transaction can be properly rolled back.
/// </summary>
[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<SqlException>(() =>
{
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);
}
}

/// <summary>
/// ExecuteScalar should work correctly when there is no error.
/// </summary>
[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);
}

/// <summary>
/// ExecuteScalar should return null when there are no rows.
/// </summary>
[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);
}

/// <summary>
/// ExecuteScalarAsync should propagate conversion errors that occur after the first result.
/// </summary>
[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<SqlException>(() => cmd.ExecuteScalarAsync());

// Assert
Assert.Equal(245, ex.Number);
Assert.Contains("Conversion failed", ex.Message);
}
finally
{
DataTestUtility.DropTable(connection, tableName);
}
}

/// <summary>
/// ExecuteScalarAsync should throw on conversion error so transaction can be properly rolled back.
/// </summary>
[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<SqlException>(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);
}
}

/// <summary>
/// ExecuteScalarAsync should work correctly when there is no error.
/// </summary>
[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);
}

/// <summary>
/// ExecuteScalarAsync should return null when there are no rows.
/// </summary>
[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);
}
}
}
Loading