[7.0 Cherry-pick] Tests | Fix TVP query hint test timeouts against shared Azure SQL DB (#4593) - #4594
Merged
cheenamalhotra merged 1 commit intoAug 28, 2026
Conversation
…ared Azure SQL DB (#4593) 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
Contributor
There was a problem hiding this comment.
Pull request overview
Ports the TVP query-hint test timeout fix to release/7.0 by reducing shared Azure SQL DDL contention.
Changes:
- Shares type and procedure setup through a class fixture.
- Raises the command timeout to 120 seconds.
- Adds parameterized existence checks and cleanup handling.
Suppressed comments (2)
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpQueryHintsTests.cs:107
- The
finallyblocks guarantee thatDropType()is invoked, but not that it succeeds. A timeout or broken connection during either drop can leave the object on the shared database, andConnection.Dispose()immediately removes the only connection available for another cleanup attempt. Since avoiding orphaned objects is the purpose of this fixture, teardown needs best-effort retry/fresh-connection cleanup rather than treating a single failed drop as complete.
try
{
DropType();
}
finally
{
Connection.Dispose();
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpQueryHintsTests.cs:87
- If
CREATE TYPEcompletes on the server butExecuteNonQuery()then reports a timeout or network error, this catch only disposes the connection. The type is left behind in the shared database—the same ambiguous-completion case this fixture is meant to handle. The outer setup cleanup should attempt the guarded type drop before disposing while preserving the original failure.
catch
{
Connection.Dispose();
throw;
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
cheenamalhotra
added a commit
to cheenamalhotra/SqlClient
that referenced
this pull request
Aug 25, 2026
… connections Captures review feedback raised on dotnet#4594 against TvpQueryHintsFixture. Both findings apply verbatim to main's copy of that fixture, and to every other consumer of the shared DatabaseObject fixture base, so they are fixed at the base rather than in one test class: - A CREATE that fails *after* the server committed it (command timeout, dropped connection) left the object behind. Creation failure now makes a best-effort drop before rethrowing the original exception. - A DROP that failed left the object orphaned forever, because names embed a GUID and the connection was disposed immediately afterwards. Dispose now retries once on a reconnected connection, rethrowing the original exception only if the retry also fails. The retry closes and reopens the existing SqlConnection rather than building a new one from its connection string: Persist Security Info defaults to false, so the password is no longer readable once the connection has been opened. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2327ee86-547d-468a-815a-04146f5fd7a5
paulmedynski
approved these changes
Aug 26, 2026
cheenamalhotra
enabled auto-merge (squash)
August 26, 2026 16:26
mdaigle
approved these changes
Aug 26, 2026
cheenamalhotra
disabled auto-merge
August 28, 2026 03:31
cheenamalhotra
deleted the
dev/automation/port-tvp-query-hints-timeouts-7.0
branch
August 28, 2026 03:31
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.
Port of #4593 to
release/7.0.Fixes the
TvpQueryHintsTests"Execution Timeout Expired" failures seen in thesqlclient_manual_azure_*legs.Problem
TvpQueryHintsTestscreated its table type and stored procedure in the constructor and dropped them inDispose. xUnit constructs the class once per test, so the 5 tests issued 20 DDL statements against the shared test database. On Azure SQL Database the resulting schema-modification lock contention pushedCREATE/DROP TYPEandCREATE/DROP PROCEDUREpast the default 30 second command timeout.Fix
IClassFixture— 20 DDL statements down to 4.The drop guards use
TYPE_IDfor the user-defined type andOBJECT_IDfor the procedure. User-defined types live insys.typesrather thansys.objects, soOBJECT_IDnever resolves them — using it there silently skips the drop and leaks the type. Both lookups are parameterized.Scope note
release/7.0does not contain the RAIIUserDefinedTypefixture introduced in #4050, so theOBJECT_ID->TYPE_IDfixture fix and the fixture parameterization changes from #4593 do not apply to this branch. ItsTable/DatabaseUser/ServerLoginfixtures are already parameterized. Only the DDL-volume fix is ported.release/6.1andrelease/6.0are not affected: they have noTvpQueryHintsTests.cs, and the query-hint scenarios there run inside a single privateQueryHintsTest()method inTvpTest.csthat creates the type and procedure once, so there is no DDL amplification.Validation
Verified against SQL Server 2022:
QHinttypes or procedures remain after the run.