Connect-DbaInstance - Open connection to get CurrentDatabase

[andreasjordan]

Type of Change

• [x] Bug fix (non-breaking change, fixes #9307)
• [ ] New feature (non-breaking change, adds functionality, fixes # )
• [ ] Breaking change (affects multiple commands or functionality, fixes # )
• [ ] Ran manual Pester test and has passed (.\tests\manual.pester.ps1)
• [ ] Adding code coverage to existing functionality
• [ ] Pester test is included
• [ ] If new file reference added for test, has is been added to github.com/dataplat/appveyor-lab ?
• [ ] Unit test is included
• [ ] Documentation
• [ ] Build system

Purpose

To be able to reliably evaluate property CurrentDatabase, the connection must first be opened.

See #9307 for more details.

[andreasjordan]

No idea why the datamasking tests fail. Do you need more details on this pull request?

[wsmelton]

I don't have an instance at work to test this but curious what affect (with your fix) does it have using a DedicatedAdminConnection ?

And then can we ensure at most doing a connection to Azure SQL still functions correctly? (just to double-check)

[andreasjordan]

Should not have any effect on DAC as DAC is always NonPooled.

Will test Azure SQL now...

[andreasjordan]

Works on Azure as well. Here I have some screenshots.

Old version:

Because of the empty $inputObject.ConnectionContext.CurrentDatabase a copy of the connection context is triggered - but both connection go to the same database: master.

New version:

Now the connection is opened during the second run of Connect-DbaInstance and the $inputObject.ConnectionContext.CurrentDatabase is not empty but filled with the correct value "master". So it is the correct database and no copy of the connection context is triggered.

Also did some query tests - everything works fine.

[andreasjordan]

I can reproduce the failing test on my local lab. Will analyse the issue and add additional commits to this branch.

[andreasjordan]

So this is really strange.

https://github.com/dataplat/dbatools/blob/95c96446d67abee6ec3ea7078407376e45908709/public/Invoke-DbaDbDataMasking.ps1#L339

This refresh just clears the SMO at this point, so .Columns is empty resulting in an invalid SQL statement some lines later. This issue was not seen without my change to Connect-DbaInstance, because the SMO would be rebuild because of some calls to Connect-DbaInstance later.

If we force a NonPooled connection, the SMO is not cleared. So this issue might be related to the way SMO handles the connection pooling. As the data masking commands are not a central part of the module, I would like to leave it like this and use a non pooled connection to solve the issue.

[andreasjordan]

Here is code that shows the problem:

$db = New-DbaDatabase -SqlInstance SQL01 -Name test
$db.Query('CREATE TABLE test (id int)')
$db.Tables['test'].Columns.Name      # shows id
Invoke-DbaQuery -SqlInstance $db.Parent -Database $db.Name -Query 'ALTER TABLE test ADD id2 int'
$db.Tables['test'].Columns.Name      # shows id
$db.Tables['test'].Columns.Refresh()
$db.Tables['test'].Columns.Name      # shows nothing
$null = Remove-DbaDatabase -SqlInstance $db.Parent -Database $db.Name -Confirm:$false

Here is code that works as expected:

$db = New-DbaDatabase -SqlInstance SQL01 -Name test
$db.Query('CREATE TABLE test (id int)')
$db.Tables['test'].Columns.Name      # shows id
$db.Query('ALTER TABLE test ADD id2 int')
$db.Tables['test'].Columns.Name      # shows id
$db.Tables['test'].Columns.Refresh()
$db.Tables['test'].Columns.Name      # shows id and id2
$null = Remove-DbaDatabase -SqlInstance $db.Parent -Database $db.Name -Confirm:$false

So the problem is somewhere inside of Invoke-DbaQuery.

I will try to find some time in the next days to get the root cause.

[andreasjordan]

While analyzing, I discovered a bug that was introduced with #8764 into Invoke-DbaQuery.

$startedWithAnOpenConnection is only $true if not only $server is a SMO, but it also have the correct database context. If the target database context is different, we run Connect-DbaInstance which copies the connection context.

In this case, $null = $server | Disconnect-DbaInstance -Verbose:$false is executed and thus the initial $server is disconnected even we want to use it in the future. And with a disconnected connection, the .Refresh() just empties the SMO in that case.

This works:

$db = New-DbaDatabase -SqlInstance SQL01 -Name test1
$db.Query('CREATE TABLE test (id int)')
$db.Tables['test'].Columns.Name      # shows id
Invoke-DbaQuery -SqlInstance $db.Parent -Database $db.Name -Query 'ALTER TABLE test ADD id2 int' -Debug  # debug says: [Disconnect-DbaInstance] removing from connection hash
$db.Parent.ConnectionContext.Connect()   # reconnect the disconnected session
$db.Tables['test'].Columns.Name      # shows id
$db.Tables['test'].Columns.Refresh()
$db.Tables['test'].Columns.Name      # shows id, id2
$null = Remove-DbaDatabase -SqlInstance $db.Parent -Database $db.Name -Confirm:$false

I have seen another issue in Invoke-DbaQuery - so I will open an issue for that and provide a fix for that first. Then I will come back to this pull request again (probably open a new one).

[andreasjordan]

Sorry about the back and forth on this pull requst. My initial aproch was not good and had some side effects. Now I optimized the code to only do the necessary steps. So only if the database context needs to be checked and only if the connection is closed and thus the CurrentDatabase is empty, we open the connection to get the value.

This solves the initial issue, test on DataMasking pass now without changing the code there and it works on Azure SQL Database as well without problems.

[potatoqualitee]

I very much appreciate the time you took to investigate this, thank you. that was so interesting, and only one command across a whole suite! amazing. the fix looks perfect in scope 🙏🏼