Tests | Fix TVP query hint test timeouts against shared Azure SQL DB - #4593
Merged
Conversation
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>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses intermittent ManualTests timeouts in the Azure SQL CI legs by reducing schema-modification (DDL) contention and ensuring transient user-defined table types are actually cleaned up after tests complete.
Changes:
- Fix
UserDefinedTypecleanup to correctly detect existing types usingTYPE_ID()(instead ofOBJECT_ID()), preventing leaked table types. - Refactor
TvpQueryHintsTeststo use anIClassFixtureso the table type and stored procedure are created/dropped once per class (instead of per test), and increase command timeout to tolerate shared-DB contention.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpQueryHintsTests.cs | Introduces a class fixture to reuse a single UDT + stored procedure across all tests, reducing DDL operations and timeouts. |
| src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/UserDefinedType.cs | Fixes drop logic to correctly locate user-defined types via TYPE_ID() so they are actually removed from the shared test database. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
cheenamalhotra
added
the
Area\Tests
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>
Member
Author
|
The Worth noting the same build confirms the fix: 13 of the 14 Azure legs passed, and every /azp run sqlclient-pr |
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
cheenamalhotra
force-pushed
the
dev/cheena/bookish-guacamole
branch
from
August 25, 2026 08:46
23382c6 to
a633e89
Compare
paulmedynski
approved these changes
Aug 25, 2026
priyankatiwari08
approved these changes
Aug 25, 2026
cheenamalhotra
added a commit
that referenced
this pull request
Aug 28, 2026
…ared Azure SQL DB (#4593) (#4594) Port of #4593 (main) adapted to release/7.0. TvpQueryHintsTests created its table type and stored procedure in the constructor and dropped them in Dispose, so the 5 tests in the class issued 20 DDL statements against the shared test database. On Azure SQL Database the resulting schema-modification lock contention pushed CREATE/DROP TYPE and CREATE/DROP PROCEDURE past the default 30 second command timeout, surfacing as sporadic 'Execution Timeout Expired' failures in the manual test legs. Create the type and procedure once per class via an IClassFixture (20 DDL statements -> 4) and raise the command timeout to 120s to absorb the contention that remains. The drop guards use TYPE_ID for the user-defined type and OBJECT_ID for the procedure. User-defined types live in sys.types rather than sys.objects, so OBJECT_ID never resolves them; using it there would silently skip the drop and leak the type into the shared database. Both lookups are parameterized. Note: release/7.0 does not contain the RAII UserDefinedType fixture from #4050, so the OBJECT_ID/TYPE_ID fix and the fixture parameterization changes in #4593 do not apply to this branch; only the DDL-volume fix is ported. Verified against SQL Server 2022: all 5 tests pass, zero orphaned QHint types or procedures remain, and the class runs ~2x faster (131ms -> 64ms) even without contention. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: cfc64bc6-a9e6-490d-88b1-4b78d25aa103
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The
sqlclient_manual_azure_*legs ofsqlclient-prhave been failing intermittently withExecution Timeout ExpiredinTvpQueryHintsTests:The timeouts always land on the
CREATE TYPE/CREATE PROC/DROP PROC; DROP TYPEstatements — never on a TVP data path.The failures also appear across many unrelated PRs, and only ever in the Azure SQL legs.
Root cause
Two independent issues combined:
UserDefinedType.DropObjectnever dropped anything. It guarded theDROPwithOBJECT_ID(...), but user-defined types live insys.types, notsys.objects, soOBJECT_IDalways returnsNULLand theDROPwas silently skipped. Every test run has been orphaning its table types into the target database. Locally, a single pass of the UDT-using manual tests leaked 108 types. In CI these accumulate indefinitely against a shared database, making metadata operations progressively slower — which matches the gradual onset around Aug 20.TvpQueryHintsTestsissued 20 DDL statements for 5 tests. Each test constructor created and dropped an identically shaped table type and stored procedure, multiplying schema-modification lock contention on a database that several platform legs hit concurrently.Fix
OBJECT_ID→TYPE_IDinUserDefinedType.DropObject, so types are actually dropped.TvpQueryHintsTestsnow uses anIClassFixtureto create the type and procedure once per class (5x less DDL), with a 120s command timeout on the fixture connection to absorb remaining contention.Validation
Run against SQL Server 2022:
TvpQueryHintsTestspass.DateTimeVariantTests,ConnectionSchemaTest,UdtDateTimeOffsetTest,ParametersTest,TvpQueryHintsTests): 357 passed. The 4 failures aretimebulk-copy cases that fail identically on baselinemain— pre-existing, in a class already tagged[Trait("Category", "flaky")].Follow-up (not in this PR)
The shared CI databases already hold a backlog of orphaned types from before this fix. Those should be purged separately, ideally during a quiet window since
DROP TYPEtakes a Sch-M lock.Checklist