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 6d536d7550..be68e649dd 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,16 @@ 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. + // 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 545a19a165..0efc5ec017 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,121 @@ 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 { /// - /// Tests for TVP query hints (sort order, uniqueness, default columns). + /// Creates the table type and stored procedure used by exactly + /// once for the whole test class. /// - [Trait("Set", "3")] - public sealed class TvpQueryHintsTests : IDisposable + /// + /// 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 { - private readonly SqlConnection _conn; - private readonly SqlCommand _cmd; - private readonly SqlParameter _param; - private readonly string _procName; - private readonly string _typeName; + /// + /// 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 TvpQueryHintsTests() + public SqlConnection Connection { get; } + + public string ProcedureName => _procedure.Name; + + public TvpQueryHintsFixture() { - 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(" + SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString) + { + CommandTimeout = CommandTimeoutSeconds + }; + + Connection = new SqlConnection(builder.ConnectionString); + + // 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)", - _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(); + + " 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; + } + } - _cmd = new SqlCommand(createTypeSql, _conn); - _cmd.ExecuteNonQuery(); + public void 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(); + } + } + } + } - _cmd.CommandText = createProcSql; - _cmd.ExecuteNonQuery(); + /// + /// Tests for TVP query hints (sort order, uniqueness, default columns). + /// + [Trait("Set", "3")] + public sealed class TvpQueryHintsTests : IClassFixture, IDisposable + { + private readonly SqlCommand _cmd; + private readonly SqlParameter _param; - _cmd.CommandText = _procName; - _cmd.CommandType = CommandType.StoredProcedure; + public TvpQueryHintsTests(TvpQueryHintsFixture fixture) + { + _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()