Connect-DbaInstance / Invoke-DbaQuery - Do not close connections of the caller and do not reuse them for the wrong database - #10564
Merged
Conversation
Commands that connect and clean up afterwards had no way to tell whether Connect-DbaInstance opened a connection for them or handed back the object the caller passed in. They inferred it from their own parameters, which is wrong whenever Connect-DbaInstance has no reason to copy anything, and then they closed a connection that belongs to the caller - taking the session, its temp tables and its database context with it. Connect-DbaInstance now writes that information into the variable behind -IsNewConnectionReference, and Invoke-DbaQuery only closes the connection when it opened one itself. A reference is used instead of a variable name because a variable set via $PSCmdlet.SessionState never reaches a caller inside of dbatools: both share the module session state, so the value lands in the local scope of Connect-DbaInstance instead of the scope of the calling command. See #10554 (do Connect-DbaInstance, Invoke-DbaQuery) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… it was opened with
To decide whether the connection that was passed in can be reused, the
command compared ConnectionContext.DatabaseName with -Database. That
property only holds the database the connection was opened with. As soon as
anything runs a USE on that connection, the two differ, the connection is
reused although it is on another database now, and the query silently runs
in the wrong database:
$server = Connect-DbaInstance -SqlInstance $instance -Database tempdb -NonPooledConnection
$null = $server.ConnectionContext.ExecuteNonQuery("USE [master]")
Invoke-DbaQuery -SqlInstance $server -Database tempdb -Query "SELECT DB_NAME()"
# master
ConnectionContext.CurrentDatabase is the database the connection is on right
now, and it is also what Connect-DbaInstance compares, so both commands now
ask the same question. That also saves a needless trip through
Connect-DbaInstance when the connection is already on the wanted database.
See #10554
(do Invoke-DbaQuery)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 12, 2026
Member
|
thank you! |
This was referenced Aug 13, 2026
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.
Replaces #10559 and #10563, which have to ship together. Fixes the first part of #10554.
Both changes are about the connection a caller hands to
Invoke-DbaQuery: the command must not close a connection it did not open, and it must not reuse one that is not on the requested database. Separately, each of them makes things worse, which is what this pull request is really about - see "Why they belong together" below.1. Connect-DbaInstance reports whether it opened a connection
A command that connects and cleans up afterwards had no way to tell whether
Connect-DbaInstanceopened a connection for it or handed back the object it was given.Invoke-DbaQueryinferred it from its own parameters and closed the connection of the caller, taking the session with it:Connect-DbaInstancenow reports this through-IsNewConnectionReference, andInvoke-DbaQueryonly disconnects when it opened the connection itself. The value is the$isNewConnectionthe command already computes internally, so no connection logic changed, only its exposure - at both places that emit a server, including the-SqlConnectionOnlypath.It takes a
[ref]and not a variable name because$PSCmdlet.SessionState.PSVariable.Set()never reaches a caller inside of dbatools: both share the module session state, so the value lands in the local scope ofConnect-DbaInstance.Set-Variable -Scope 1does not help either, because from a module function the parent scope is the module scope.2. Invoke-DbaQuery tests the database the connection is on
To decide whether the connection can be reused, the command compared
ConnectionContext.DatabaseName- the database the connection was opened with - against-Database.Connect-DbaInstanceasks the same question withConnectionContext.CurrentDatabase, the database the connection is on right now. The two differ as soon as anything runs aUSE, and the query then runs in the wrong database:Why they belong together
#10559 alone breaks
Install-DbaFirstResponderKit, which is how this was found - its test file failed in CI and reproduces locally.The command opens one connection with
-Database $Database, and before signing the procedures it runsGet-DbaLogin | Remove-DbaLogin. SMO'sLogin.Drop()leaves that shared connection inmaster(New-DbaLogindoes the same,Get-DbaLogindoes not) - the defect described in #10555, here from SMO's own object scripting rather than from$db.Query(). Every later query for the user database is then reused onto a connection that sits in master, and signing fails withCannot alter the object 'sp_BlitzFirst', because it does not exist or you do not have permission.Today that is hidden: the disconnect from part 1 closes the connection in between, and the reconnect lands back on the catalog from the connection string. Measured on the three states:
So part 1 removes a crutch and part 2 removes the need for it.
Install-DbaFirstResponderKitpasses 22 of 22 with both, and the install produces its 2 signed objects again.Tests
Connect-DbaInstance: the parameter list, and that the reference is$truefor a string,$falsewhen a server object is passed back in,$truewhen the connection context has to be copied, and that the same object comes back when nothing has to change.Invoke-DbaQuery: the connection of the caller stays open and its temp table survives; connections the command opens itself are still closed, so #6210 does not come back; the connection is still reused while it is on the requested database; and a query runs in the requested database after the connection was moved away from it.Reverting either production change makes the matching test fail, so both are covered by something that actually tests them.
Locally against SQL Server 2025 and 2022:
Connect-DbaInstance32 passed and 1 skipped (Azure),Invoke-DbaQuery31 passed,Install-DbaFirstResponderKit22 passed.Still worth checking in review
-ReadOnlySqlConnection, connection strings and registered serversThis text was created by Claude and reviewed by Andreas Jordan.