Skip to content

Commit 3ac4434

Browse files
samsharma2700samsharma2700
committed
Port #3912 to release/6.1
Backport of PR #3912 (Fix #3736 | Propagate Errors from ExecuteScalar) to release/6.1 branch. Changes: - netcore/netfx SqlCommand.cs: Drain remaining results in CompleteExecuteScalar sync path - netcore/netfx SqlCommand.cs: Remove _batchRPCMode guard in ExecuteScalarUntilEndAsync - MultipleResultsTest.cs: Updated to expect SqlException from ExecuteScalar draining - SqlCommandExecuteScalarTest.cs: New regression tests (sync/async, transactions, multi-result) - ManualTesting.Tests.csproj: Include new test file Fixes #3736
1 parent cfaf082 commit 3ac4434

5 files changed

Lines changed: 340 additions & 7 deletions

File tree

‎src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1176,6 +1176,12 @@ private object CompleteExecuteScalar(SqlDataReader ds, bool returnLastResult)
11761176
}
11771177
}
11781178
} while (returnLastResult && ds.NextResult());
1179+
1180+
// Drain any remaining results to ensure errors are propagated
1181+
if (!returnLastResult)
1182+
{
1183+
while (ds.NextResult()) { }
1184+
}
11791185
}
11801186
finally
11811187
{
@@ -3001,7 +3007,7 @@ private async Task<object> ExecuteScalarUntilEndAsync(SqlDataReader reader, Canc
30013007
retval = reader.GetValue(0); // no async untyped value getter, this will work ok as long as the value is in the current packet
30023008
}
30033009
}
3004-
while (_batchRPCMode && !cancellationToken.IsCancellationRequested && await reader.NextResultAsync(cancellationToken).ConfigureAwait(false));
3010+
while (!cancellationToken.IsCancellationRequested && await reader.NextResultAsync(cancellationToken).ConfigureAwait(false));
30053011
return retval;
30063012
}
30073013

‎src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1371,7 +1371,7 @@ private async Task<object> ExecuteScalarUntilEndAsync(SqlDataReader reader, Canc
13711371
retval = reader.GetValue(0); // no async untyped value getter, this will work ok as long as the value is in the current packet
13721372
}
13731373
}
1374-
while (_batchRPCMode && !cancellationToken.IsCancellationRequested && await reader.NextResultAsync(cancellationToken).ConfigureAwait(false));
1374+
while (!cancellationToken.IsCancellationRequested && await reader.NextResultAsync(cancellationToken).ConfigureAwait(false));
13751375
return retval;
13761376
}
13771377

@@ -1391,6 +1391,12 @@ private object CompleteExecuteScalar(SqlDataReader ds, bool returnLastResult)
13911391
}
13921392
}
13931393
} while (returnLastResult && ds.NextResult());
1394+
1395+
// Drain any remaining results to ensure errors are propagated
1396+
if (!returnLastResult)
1397+
{
1398+
while (ds.NextResult()) { }
1399+
}
13941400
}
13951401
finally
13961402
{

‎src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,7 @@
301301
<Compile Include="SQL\Common\SystemDataInternals\TdsParserStateObjectHelper.cs" />
302302
<Compile Include="SQL\ConnectionTestWithSSLCert\CertificateTest.cs" />
303303
<Compile Include="SQL\ConnectionTestWithSSLCert\CertificateTestWithTdsServer.cs" />
304+
<Compile Include="SQL\SqlCommand\SqlCommandExecuteScalarTest.cs" />
304305
<Compile Include="SQL\SqlCommand\SqlCommandStoredProcTest.cs" />
305306
<Compile Include="XUnitAssemblyAttributes.cs" />
306307
</ItemGroup>

‎src/Microsoft.Data.SqlClient/tests/ManualTests/ProviderAgnostic/MultipleResultsTest/MultipleResultsTest.cs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,10 @@ public void ExecuteScalar()
8787

8888
command.CommandText = s_sqlStatement;
8989

90-
// ExecuteScalar will select the first result set and the info message preceding it, then stop.
91-
command.ExecuteScalar();
92-
Assert.True(messages.TryDequeue(out string lastMessage));
93-
Assert.Empty(messages);
94-
Assert.Equal(ResultSet1_Message, lastMessage);
90+
// ExecuteScalar now drains all result sets to ensure errors are not silently ignored (GH #3736 fix).
91+
// Since the SQL statement contains RAISERRORs after the first result set, an exception is thrown.
92+
SqlException ex = Assert.Throws<SqlException>(() => command.ExecuteScalar());
93+
Assert.Contains("Error 1", ex.Message);
9594
}
9695

9796
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]

‎src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs‎

Lines changed: 321 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,321 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
6+
using System.Threading.Tasks;
7+
using Xunit;
8+
9+
namespace Microsoft.Data.SqlClient.ManualTesting.Tests
10+
{
11+
public class SqlCommandExecuteScalarTest
12+
{
13+
/// <summary>
14+
/// Validates that ExecuteScalar propagates errors that occur after returning the first result.
15+
/// This tests the fix for GitHub issue #3736 where conversion errors during WHERE clause
16+
/// evaluation were being silently swallowed.
17+
/// </summary>
18+
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
19+
public void ExecuteScalar_PropagatesErrorAfterFirstResult_Sync()
20+
{
21+
string tableName = DataTestUtility.GetLongName("ES_ErrorProp");
22+
23+
using (var connection = new SqlConnection(DataTestUtility.TCPConnectionString))
24+
{
25+
connection.Open();
26+
27+
// Create table with varchar column that will cause conversion error
28+
using (var cmd = new SqlCommand($@"
29+
CREATE TABLE {tableName} (
30+
Id INT IDENTITY(1,1) PRIMARY KEY,
31+
Val VARCHAR(50)
32+
);
33+
INSERT INTO {tableName} (Val) VALUES ('100'), ('42-43'), ('200');", connection))
34+
{
35+
cmd.ExecuteNonQuery();
36+
}
37+
38+
try
39+
{
40+
// This query will return '100' first, then fail on '42-43' conversion
41+
using (var cmd = new SqlCommand($"SELECT Id FROM {tableName} WHERE Val = 12345", connection))
42+
{
43+
Assert.Throws<SqlException>(() => cmd.ExecuteScalar());
44+
}
45+
}
46+
finally
47+
{
48+
// Cleanup
49+
using (var cmd = new SqlCommand($"DROP TABLE IF EXISTS {tableName}", connection))
50+
{
51+
cmd.ExecuteNonQuery();
52+
}
53+
}
54+
}
55+
}
56+
57+
/// <summary>
58+
/// Validates that ExecuteScalarAsync propagates errors that occur after returning the first result.
59+
/// </summary>
60+
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
61+
public async Task ExecuteScalar_PropagatesErrorAfterFirstResult_Async()
62+
{
63+
string tableName = DataTestUtility.GetLongName("ES_ErrorPropAsync");
64+
65+
using (var connection = new SqlConnection(DataTestUtility.TCPConnectionString))
66+
{
67+
await connection.OpenAsync();
68+
69+
// Create table with varchar column that will cause conversion error
70+
using (var cmd = new SqlCommand($@"
71+
CREATE TABLE {tableName} (
72+
Id INT IDENTITY(1,1) PRIMARY KEY,
73+
Val VARCHAR(50)
74+
);
75+
INSERT INTO {tableName} (Val) VALUES ('100'), ('42-43'), ('200');", connection))
76+
{
77+
await cmd.ExecuteNonQueryAsync();
78+
}
79+
80+
try
81+
{
82+
// This query will return '100' first, then fail on '42-43' conversion
83+
using (var cmd = new SqlCommand($"SELECT Id FROM {tableName} WHERE Val = 12345", connection))
84+
{
85+
await Assert.ThrowsAsync<SqlException>(() => cmd.ExecuteScalarAsync());
86+
}
87+
}
88+
finally
89+
{
90+
// Cleanup
91+
using (var cmd = new SqlCommand($"DROP TABLE IF EXISTS {tableName}", connection))
92+
{
93+
await cmd.ExecuteNonQueryAsync();
94+
}
95+
}
96+
}
97+
}
98+
99+
/// <summary>
100+
/// Validates that ExecuteScalar properly propagates errors within a transaction,
101+
/// preventing the "zombie transaction" issue where the transaction appears valid
102+
/// but has actually been rolled back by the server.
103+
/// </summary>
104+
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
105+
public void ExecuteScalar_PropagatesErrorWithTransaction_Sync()
106+
{
107+
string tableName = DataTestUtility.GetLongName("ES_TxnError");
108+
109+
using (var connection = new SqlConnection(DataTestUtility.TCPConnectionString))
110+
{
111+
connection.Open();
112+
113+
// Create table with varchar column that will cause conversion error
114+
using (var cmd = new SqlCommand($@"
115+
CREATE TABLE {tableName} (
116+
Id INT IDENTITY(1,1) PRIMARY KEY,
117+
Val VARCHAR(50)
118+
);
119+
INSERT INTO {tableName} (Val) VALUES ('100'), ('42-43'), ('200');", connection))
120+
{
121+
cmd.ExecuteNonQuery();
122+
}
123+
124+
try
125+
{
126+
using (var transaction = connection.BeginTransaction())
127+
{
128+
// This query will return '100' first, then fail on '42-43' conversion
129+
using (var cmd = new SqlCommand($"SELECT Id FROM {tableName} WHERE Val = 12345", connection, transaction))
130+
{
131+
Assert.Throws<SqlException>(() => cmd.ExecuteScalar());
132+
}
133+
134+
// Transaction should still be usable to rollback
135+
transaction.Rollback();
136+
}
137+
}
138+
finally
139+
{
140+
// Cleanup
141+
using (var cmd = new SqlCommand($"DROP TABLE IF EXISTS {tableName}", connection))
142+
{
143+
cmd.ExecuteNonQuery();
144+
}
145+
}
146+
}
147+
}
148+
149+
/// <summary>
150+
/// Validates that ExecuteScalarAsync properly propagates errors within a transaction.
151+
/// </summary>
152+
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
153+
public async Task ExecuteScalar_PropagatesErrorWithTransaction_Async()
154+
{
155+
string tableName = DataTestUtility.GetLongName("ES_TxnErrorAsync");
156+
157+
using (var connection = new SqlConnection(DataTestUtility.TCPConnectionString))
158+
{
159+
await connection.OpenAsync();
160+
161+
// Create table with varchar column that will cause conversion error
162+
using (var cmd = new SqlCommand($@"
163+
CREATE TABLE {tableName} (
164+
Id INT IDENTITY(1,1) PRIMARY KEY,
165+
Val VARCHAR(50)
166+
);
167+
INSERT INTO {tableName} (Val) VALUES ('100'), ('42-43'), ('200');", connection))
168+
{
169+
await cmd.ExecuteNonQueryAsync();
170+
}
171+
172+
try
173+
{
174+
using (var transaction = connection.BeginTransaction())
175+
{
176+
// This query will return '100' first, then fail on '42-43' conversion
177+
using (var cmd = new SqlCommand($"SELECT Id FROM {tableName} WHERE Val = 12345", connection, transaction))
178+
{
179+
await Assert.ThrowsAsync<SqlException>(() => cmd.ExecuteScalarAsync());
180+
}
181+
182+
// Transaction should still be usable to rollback
183+
transaction.Rollback();
184+
}
185+
}
186+
finally
187+
{
188+
// Cleanup
189+
using (var cmd = new SqlCommand($"DROP TABLE IF EXISTS {tableName}", connection))
190+
{
191+
await cmd.ExecuteNonQueryAsync();
192+
}
193+
}
194+
}
195+
}
196+
197+
/// <summary>
198+
/// Sanity test to ensure ExecuteScalar still works correctly for normal queries.
199+
/// </summary>
200+
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
201+
public void ExecuteScalar_ReturnsFirstValue_Sync()
202+
{
203+
using (var connection = new SqlConnection(DataTestUtility.TCPConnectionString))
204+
{
205+
connection.Open();
206+
207+
using (var cmd = new SqlCommand("SELECT 42", connection))
208+
{
209+
object result = cmd.ExecuteScalar();
210+
Assert.Equal(42, result);
211+
}
212+
}
213+
}
214+
215+
/// <summary>
216+
/// Sanity test to ensure ExecuteScalarAsync still works correctly for normal queries.
217+
/// </summary>
218+
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
219+
public async Task ExecuteScalar_ReturnsFirstValue_Async()
220+
{
221+
using (var connection = new SqlConnection(DataTestUtility.TCPConnectionString))
222+
{
223+
await connection.OpenAsync();
224+
225+
using (var cmd = new SqlCommand("SELECT 42", connection))
226+
{
227+
object result = await cmd.ExecuteScalarAsync();
228+
Assert.Equal(42, result);
229+
}
230+
}
231+
}
232+
233+
/// <summary>
234+
/// Test ExecuteScalar with multiple result sets to ensure all are processed.
235+
/// </summary>
236+
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
237+
public void ExecuteScalar_ProcessesMultipleResultSets_Sync()
238+
{
239+
string tableName = DataTestUtility.GetLongName("ES_MultiResult");
240+
241+
using (var connection = new SqlConnection(DataTestUtility.TCPConnectionString))
242+
{
243+
connection.Open();
244+
245+
// Create table with varchar column that will cause conversion error in second result set
246+
using (var cmd = new SqlCommand($@"
247+
CREATE TABLE {tableName} (
248+
Id INT IDENTITY(1,1) PRIMARY KEY,
249+
Val VARCHAR(50)
250+
);
251+
INSERT INTO {tableName} (Val) VALUES ('100'), ('42-43'), ('200');", connection))
252+
{
253+
cmd.ExecuteNonQuery();
254+
}
255+
256+
try
257+
{
258+
// First result set succeeds, second result set has conversion error
259+
using (var cmd = new SqlCommand($@"
260+
SELECT 1;
261+
SELECT Id FROM {tableName} WHERE Val = 12345;", connection))
262+
{
263+
Assert.Throws<SqlException>(() => cmd.ExecuteScalar());
264+
}
265+
}
266+
finally
267+
{
268+
// Cleanup
269+
using (var cmd = new SqlCommand($"DROP TABLE IF EXISTS {tableName}", connection))
270+
{
271+
cmd.ExecuteNonQuery();
272+
}
273+
}
274+
}
275+
}
276+
277+
/// <summary>
278+
/// Test ExecuteScalarAsync with multiple result sets to ensure all are processed.
279+
/// </summary>
280+
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
281+
public async Task ExecuteScalar_ProcessesMultipleResultSets_Async()
282+
{
283+
string tableName = DataTestUtility.GetLongName("ES_MultiResultAsync");
284+
285+
using (var connection = new SqlConnection(DataTestUtility.TCPConnectionString))
286+
{
287+
await connection.OpenAsync();
288+
289+
// Create table with varchar column that will cause conversion error in second result set
290+
using (var cmd = new SqlCommand($@"
291+
CREATE TABLE {tableName} (
292+
Id INT IDENTITY(1,1) PRIMARY KEY,
293+
Val VARCHAR(50)
294+
);
295+
INSERT INTO {tableName} (Val) VALUES ('100'), ('42-43'), ('200');", connection))
296+
{
297+
await cmd.ExecuteNonQueryAsync();
298+
}
299+
300+
try
301+
{
302+
// First result set succeeds, second result set has conversion error
303+
using (var cmd = new SqlCommand($@"
304+
SELECT 1;
305+
SELECT Id FROM {tableName} WHERE Val = 12345;", connection))
306+
{
307+
await Assert.ThrowsAsync<SqlException>(() => cmd.ExecuteScalarAsync());
308+
}
309+
}
310+
finally
311+
{
312+
// Cleanup
313+
using (var cmd = new SqlCommand($"DROP TABLE IF EXISTS {tableName}", connection))
314+
{
315+
await cmd.ExecuteNonQueryAsync();
316+
}
317+
}
318+
}
319+
}
320+
}
321+
}

0 commit comments

Comments
 (0)