From f373c7b4b37f1be6814ceb6afef728f9a1172660 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Mon, 24 Aug 2026 23:31:11 -0700 Subject: [PATCH 1/3] Tests | Fix TVP query hint test timeouts against shared Azure SQL DB The `sqlclient_manual_azure_*` legs have been failing intermittently with "Execution Timeout Expired" while executing the CREATE/DROP TYPE and CREATE/DROP PROCEDURE statements in TvpQueryHintsTests. Two independent issues combined to cause this: 1. `UserDefinedType.DropObject` guarded its DROP with `OBJECT_ID(...)`. User-defined types live in `sys.types`, not `sys.objects`, so `OBJECT_ID` always returns NULL for them and the DROP was silently skipped. Every test run therefore orphaned its table types in the shared test database. Locally, a single pass of the UDT-using manual tests leaked 108 types; in CI these accumulate indefinitely and make subsequent metadata operations progressively slower. Switched to `TYPE_ID(...)`, which is the correct lookup. 2. TvpQueryHintsTests created and dropped an identically shaped table type and stored procedure in every test constructor, issuing 20 DDL statements for 5 tests. Moved that setup into a class fixture so it happens once, and gave the fixture connection a longer default command timeout to absorb the schema-modification lock contention that remains when several platform legs run against the same Azure SQL database concurrently. Verified against SQL Server 2022: all 5 TvpQueryHintsTests pass and the UDT-using manual tests now leak zero types (previously 108). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../DatabaseObjects/UserDefinedType.cs | 5 +- .../SQL/ParameterTest/TvpQueryHintsTests.cs | 102 +++++++++++------- 2 files changed, 70 insertions(+), 37 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/UserDefinedType.cs b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/UserDefinedType.cs index 6d536d7550..8d469ae47f 100644 --- a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/UserDefinedType.cs +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/UserDefinedType.cs @@ -34,7 +34,10 @@ protected override void CreateObject(string definition) protected override void DropObject() { - using SqlCommand dropCommand = new($"IF (OBJECT_ID('{Name}') IS NOT NULL) DROP TYPE {Name}", Connection); + // NOTE: User-defined types live in sys.types, not sys.objects, so OBJECT_ID() always + // returns NULL for them. Using it here silently skipped every drop and leaked the type + // into the (shared) test database. TYPE_ID() is the correct lookup. + using SqlCommand dropCommand = new($"IF (TYPE_ID('{Name}') IS NOT NULL) DROP TYPE {Name}", Connection); dropCommand.ExecuteNonQuery(); } diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpQueryHintsTests.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpQueryHintsTests.cs index 545a19a165..5465c2abef 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpQueryHintsTests.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpQueryHintsTests.cs @@ -6,58 +6,88 @@ using System.Collections.Generic; using System.Data; using Microsoft.Data.SqlClient.Server; +using Microsoft.Data.SqlClient.Tests.Common.Fixtures.DatabaseObjects; using Xunit; namespace Microsoft.Data.SqlClient.ManualTesting.Tests { + /// + /// Creates the table type and stored procedure used by exactly + /// once for the whole test class. + /// + /// + /// Every test in the class needs an identically shaped table type and procedure, so creating them + /// per test only multiplies the amount of DDL issued against the (shared) test database. On Azure + /// SQL Database the resulting schema-modification lock contention was enough to push + /// CREATE/DROP TYPE and CREATE/DROP PROCEDURE past the default 30 second command timeout, which + /// showed up as sporadic "Execution Timeout Expired" failures in the manual test legs. Sharing a + /// single type/procedure via a class fixture cuts the DDL statement count by 5x, and the extended + /// command timeout below absorbs the contention that remains. + /// + public sealed class TvpQueryHintsFixture : IDisposable + { + /// + /// Command timeout (in seconds) applied to every command issued on . + /// Deliberately generous: DDL against a shared Azure SQL Database can block for a long time + /// behind concurrently executing test legs. + /// + private const int CommandTimeoutSeconds = 120; + + private readonly UserDefinedType _tableType; + private readonly StoredProcedure _procedure; + + public SqlConnection Connection { get; } + + public string ProcedureName => _procedure.Name; + + public TvpQueryHintsFixture() + { + SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString) + { + CommandTimeout = CommandTimeoutSeconds + }; + + Connection = new SqlConnection(builder.ConnectionString); + Connection.Open(); + + _tableType = new UserDefinedType(Connection, "QHint", + "TABLE(" + + " c1 Int DEFAULT -1," + + " c2 NVarChar(40) DEFAULT N'DEFUALT'," + + " c3 DateTime DEFAULT '1/1/2006'," + + " c4 Int DEFAULT -1)"); + + _procedure = new StoredProcedure(Connection, "QHint_Proc", + $"(@tvp {_tableType.Name} READONLY) AS SELECT TOP(2) * FROM @tvp ORDER BY c1"); + } + + public void Dispose() + { + _procedure.Dispose(); + _tableType.Dispose(); + Connection.Dispose(); + } + } + /// /// Tests for TVP query hints (sort order, uniqueness, default columns). /// [Trait("Set", "3")] - public sealed class TvpQueryHintsTests : IDisposable + public sealed class TvpQueryHintsTests : IClassFixture, IDisposable { - private readonly SqlConnection _conn; private readonly SqlCommand _cmd; private readonly SqlParameter _param; - private readonly string _procName; - private readonly string _typeName; - public TvpQueryHintsTests() + public TvpQueryHintsTests(TvpQueryHintsFixture fixture) { - Guid randomizer = Guid.NewGuid(); - _typeName = string.Format("dbo.[QHint_{0}]", randomizer); - _procName = string.Format("dbo.[QHint_Proc_{0}]", randomizer); - string createTypeSql = string.Format( - "CREATE TYPE {0} AS TABLE(" - + " c1 Int DEFAULT -1," - + " c2 NVarChar(40) DEFAULT N'DEFUALT'," - + " c3 DateTime DEFAULT '1/1/2006'," - + " c4 Int DEFAULT -1)", - _typeName); - string createProcSql = string.Format( - "CREATE PROC {0}(@tvp {1} READONLY) AS SELECT TOP(2) * FROM @tvp ORDER BY c1", _procName, _typeName); - - _conn = new SqlConnection(DataTestUtility.TCPConnectionString); - _conn.Open(); - - _cmd = new SqlCommand(createTypeSql, _conn); - _cmd.ExecuteNonQuery(); - - _cmd.CommandText = createProcSql; - _cmd.ExecuteNonQuery(); - - _cmd.CommandText = _procName; - _cmd.CommandType = CommandType.StoredProcedure; + _cmd = new SqlCommand(fixture.ProcedureName, fixture.Connection) + { + CommandType = CommandType.StoredProcedure + }; _param = _cmd.Parameters.Add("@tvp", SqlDbType.Structured); } - public void Dispose() - { - string dropSql = string.Format("DROP PROC {0}; DROP TYPE {1}", _procName, _typeName); - using SqlCommand cmd = new(dropSql, _conn); - cmd.ExecuteNonQuery(); - _conn.Dispose(); - } + public void Dispose() => _cmd.Dispose(); [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))] public void SortOrderSimple() From a8e8d93844cf89b553cb043d41778bba08dfb65b Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Mon, 24 Aug 2026 23:53:19 -0700 Subject: [PATCH 2/3] Tests | Parameterize object-name lookups in database object fixtures Addresses review feedback on the TYPE_ID() guard, and applies the same fix to the sibling fixtures that had the identical pattern. Generated object names embed Environment.UserName and Environment.MachineName (DatabaseObject.GenerateLongName), so interpolating them into a T-SQL string literal breaks the batch if either contains an apostrophe, silently skipping the drop and leaking the object. Pass the name as a parameter instead, matching what ColumnEncryptionKey and ColumnMasterKey already do. The identifiers in the DROP statements stay safe because GenerateLongName bracket-quotes them. Covers UserDefinedType (TYPE_ID), Table and StoredProcedure (OBJECT_ID), DatabaseUser (USER_ID) and ServerLogin (SUSER_ID). Also makes TvpQueryHintsFixture exception-safe: a failure partway through the constructor previously orphaned the table type and the connection, and a transient error while dropping the procedure skipped the type and connection cleanup entirely - the exact leak this change set exists to prevent. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Fixtures/DatabaseObjects/DatabaseUser.cs | 8 ++- .../Fixtures/DatabaseObjects/ServerLogin.cs | 8 ++- .../DatabaseObjects/StoredProcedure.cs | 8 ++- .../Common/Fixtures/DatabaseObjects/Table.cs | 8 ++- .../DatabaseObjects/UserDefinedType.cs | 8 ++- .../SQL/ParameterTest/TvpQueryHintsTests.cs | 59 +++++++++++++++---- 6 files changed, 81 insertions(+), 18 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseUser.cs b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseUser.cs index bc94bc78ef..a757e9c9e0 100644 --- a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseUser.cs +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseUser.cs @@ -35,7 +35,13 @@ protected override void CreateObject(string definition) protected override void DropObject() { - using SqlCommand dropCommand = new($"IF USER_ID('{UnescapedName}') IS NOT NULL DROP USER {Name}", Connection); + // NOTE: The name is passed to USER_ID() as a parameter rather than being interpolated into + // a string literal, because it embeds Environment.UserName/MachineName (see + // DatabaseObject.GenerateLongName) and an apostrophe in either would break the batch. + // The identifier in DROP USER is already bracket-quoted by GenerateLongName. + using SqlCommand dropCommand = new($"IF USER_ID(@name) IS NOT NULL DROP USER {Name}", Connection); + + dropCommand.Parameters.AddWithValue("@name", UnescapedName); ExecuteCommandInDatabase(dropCommand); } diff --git a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs index 154899e9b3..ce264a9d14 100644 --- a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs @@ -92,7 +92,13 @@ protected override void CreateObject(string definition) protected override void DropObject() { - using SqlCommand dropCommand = new($"IF SUSER_ID('{UnescapedName}') IS NOT NULL DROP LOGIN {Name}", Connection); + // NOTE: The name is passed to SUSER_ID() as a parameter rather than being interpolated into + // a string literal, because it embeds Environment.UserName/MachineName (see + // DatabaseObject.GenerateLongName) and an apostrophe in either would break the batch. + // The identifier in DROP LOGIN is already bracket-quoted by GenerateLongName. + using SqlCommand dropCommand = new($"IF SUSER_ID(@name) IS NOT NULL DROP LOGIN {Name}", Connection); + + dropCommand.Parameters.AddWithValue("@name", UnescapedName); dropCommand.ExecuteNonQuery(); } diff --git a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/StoredProcedure.cs b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/StoredProcedure.cs index d1b60612d1..0ebbd6cb5e 100644 --- a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/StoredProcedure.cs +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/StoredProcedure.cs @@ -34,7 +34,13 @@ protected override void CreateObject(string definition) protected override void DropObject() { - using SqlCommand dropCommand = new($"IF (OBJECT_ID('{Name}') IS NOT NULL) DROP PROCEDURE {Name}", Connection); + // NOTE: The name is passed to OBJECT_ID() as a parameter rather than being interpolated + // into a string literal, because it embeds Environment.UserName/MachineName (see + // DatabaseObject.GenerateLongName) and an apostrophe in either would break the batch. + // The identifier in DROP PROCEDURE is already bracket-quoted by GenerateLongName. + using SqlCommand dropCommand = new($"IF (OBJECT_ID(@name) IS NOT NULL) DROP PROCEDURE {Name}", Connection); + + dropCommand.Parameters.AddWithValue("@name", Name); dropCommand.ExecuteNonQuery(); } diff --git a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/Table.cs b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/Table.cs index 2838ae2272..42d07a8fba 100644 --- a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/Table.cs +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/Table.cs @@ -34,7 +34,13 @@ protected override void CreateObject(string definition) protected override void DropObject() { - using SqlCommand dropCommand = new($"IF (OBJECT_ID('{Name}') IS NOT NULL) DROP TABLE {Name}", Connection); + // NOTE: The name is passed to OBJECT_ID() as a parameter rather than being interpolated + // into a string literal, because it embeds Environment.UserName/MachineName (see + // DatabaseObject.GenerateLongName) and an apostrophe in either would break the batch. + // The identifier in DROP TABLE is already bracket-quoted by GenerateLongName. + using SqlCommand dropCommand = new($"IF (OBJECT_ID(@name) IS NOT NULL) DROP TABLE {Name}", Connection); + + dropCommand.Parameters.AddWithValue("@name", Name); dropCommand.ExecuteNonQuery(); } diff --git a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/UserDefinedType.cs b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/UserDefinedType.cs index 8d469ae47f..be68e649dd 100644 --- a/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/UserDefinedType.cs +++ b/src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/UserDefinedType.cs @@ -37,7 +37,13 @@ protected override void DropObject() // NOTE: User-defined types live in sys.types, not sys.objects, so OBJECT_ID() always // returns NULL for them. Using it here silently skipped every drop and leaked the type // into the (shared) test database. TYPE_ID() is the correct lookup. - using SqlCommand dropCommand = new($"IF (TYPE_ID('{Name}') IS NOT NULL) DROP TYPE {Name}", Connection); + // NOTE: The name is passed to TYPE_ID() as a parameter rather than being interpolated into + // a string literal, because it embeds Environment.UserName/MachineName (see + // DatabaseObject.GenerateLongName) and an apostrophe in either would break the batch. + // The identifier in DROP TYPE is already bracket-quoted by GenerateLongName. + using SqlCommand dropCommand = new($"IF (TYPE_ID(@typeName) IS NOT NULL) DROP TYPE {Name}", Connection); + + dropCommand.Parameters.AddWithValue("@typeName", Name); dropCommand.ExecuteNonQuery(); } diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpQueryHintsTests.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpQueryHintsTests.cs index 5465c2abef..0efc5ec017 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpQueryHintsTests.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpQueryHintsTests.cs @@ -48,24 +48,57 @@ public TvpQueryHintsFixture() }; Connection = new SqlConnection(builder.ConnectionString); - Connection.Open(); - _tableType = new UserDefinedType(Connection, "QHint", - "TABLE(" - + " c1 Int DEFAULT -1," - + " c2 NVarChar(40) DEFAULT N'DEFUALT'," - + " c3 DateTime DEFAULT '1/1/2006'," - + " c4 Int DEFAULT -1)"); - - _procedure = new StoredProcedure(Connection, "QHint_Proc", - $"(@tvp {_tableType.Name} READONLY) AS SELECT TOP(2) * FROM @tvp ORDER BY c1"); + // Partial construction must not leak the objects created so far, otherwise a transient + // failure here would orphan a type in the shared database. + try + { + Connection.Open(); + + _tableType = new UserDefinedType(Connection, "QHint", + "TABLE(" + + " c1 Int DEFAULT -1," + + " c2 NVarChar(40) DEFAULT N'DEFUALT'," + + " c3 DateTime DEFAULT '1/1/2006'," + + " c4 Int DEFAULT -1)"); + + try + { + _procedure = new StoredProcedure(Connection, "QHint_Proc", + $"(@tvp {_tableType.Name} READONLY) AS SELECT TOP(2) * FROM @tvp ORDER BY c1"); + } + catch + { + _tableType.Dispose(); + throw; + } + } + catch + { + Connection.Dispose(); + throw; + } } public void Dispose() { - _procedure.Dispose(); - _tableType.Dispose(); - Connection.Dispose(); + // Each step runs even if an earlier one fails, so a transient error while dropping the + // procedure cannot leave the type (or the connection) behind. + try + { + _procedure.Dispose(); + } + finally + { + try + { + _tableType.Dispose(); + } + finally + { + Connection.Dispose(); + } + } } } From a633e89309fb17417209c0190bcbfb4de7a3fdc2 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Tue, 25 Aug 2026 00:57:18 -0700 Subject: [PATCH 3/3] Re-run CI The required sqlclient-pr pipeline is failing only on tests unrelated to this change, in a pipeline with a ~48% baseline failure rate across other PRs. Observed failures, none of which touch any file in this PR: MARSTest.MarsScenarioClientJoin (also failed on PR #4567) SqlCommandCancelTest.TimeOutDuringRead_Tcp (timing sensitive) TransactionEnlistmentTest.TestManualEnlistment_Enlist (also failed on PR #4585) The /azp run comment trigger is not enabled on this repo, so refreshing the head SHA is the only available way to re-run the required checks. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: cfc64bc6-a9e6-490d-88b1-4b78d25aa103