Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Copy-DbaDbViewData #6355

Merged
merged 10 commits into from Feb 12, 2020
Merged

Conversation

@jaxnoth
Copy link
Contributor

jaxnoth commented Feb 10, 2020

Type of Change

  • Bug fix (non-breaking change, fixes # )
  • New feature (non-breaking change, adds functionality, fixes #6301 )
  • Breaking change (effects 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/sqlcollaborative/appveyor-lab ?
  • Nunit test is included
  • Documentation
  • Build system

Purpose

Adds a new function to allow copies of data from a view into a table

Approach

Duplicates copy-dbadbtabledata functionality to provide the same copy abilities to view data

Commands to test

See examples in command documentation

Screenshots

image

Learning

functions/Get-DbaDbView.ps1 Outdated Show resolved Hide resolved
functions/Copy-DbaDbViewData.ps1 Outdated Show resolved Hide resolved
functions/Copy-DbaDbViewData.ps1 Outdated Show resolved Hide resolved
functions/Get-DbaDbView.ps1 Outdated Show resolved Hide resolved
@potatoqualitee

This comment has been minimized.

Copy link
Member

potatoqualitee commented Feb 10, 2020

This is so good, I'm pumped! Thank you @jaxnoth 🏆

jshall and others added 2 commits Feb 10, 2020
@potatoqualitee

This comment has been minimized.

Copy link
Member

potatoqualitee commented Feb 11, 2020

I added a thing to change the table names to avoid conflicts but the tests are still failing 🤔

@jshall

This comment has been minimized.

Copy link
Contributor

jshall commented Feb 12, 2020

It appears that I've resolved all the tests. @wsmelton and @potatoqualitee, is there anything more we need to address?

Copy link
Member

wsmelton left a comment

A few more things to check on...

#select view into tempdb to generate script
$tempTableName = "$($sqlview.Name)_table"
$createquery = "SELECT * INTO tempdb..$tempTableName FROM [$($sqlview.Schema)].[$($sqlview.Name)] WHERE 1=2"
Invoke-DbaQuery -SqlInstance $server -Database $Database -Query $createquery

This comment has been minimized.

Copy link
@wsmelton

wsmelton Feb 12, 2020

Member
Suggested change
Invoke-DbaQuery -SqlInstance $server -Database $Database -Query $createquery
Invoke-DbaQuery -SqlInstance $server -Database $Database -Query $createquery -EnableException

Please update so this causes you catch block to be triggered properly.

This comment has been minimized.

Copy link
@jaxnoth

jaxnoth Feb 12, 2020

Author Contributor

Added EnableException flag

Invoke-DbaQuery -SqlInstance $server -Database $Database -Query $createquery
$tempTable = Get-DbaDbTable -SqlInstance $server -Database tempdb -Table $tempTableName
$tablescript = $tempTable | Export-DbaScript -Passthru | Out-String
Invoke-DbaQuery -SqlInstance $server -Database $Database -Query "DROP TABLE tempdb..$tempTableName"

This comment has been minimized.

Copy link
@wsmelton

wsmelton Feb 12, 2020

Member
Suggested change
Invoke-DbaQuery -SqlInstance $server -Database $Database -Query "DROP TABLE tempdb..$tempTableName"
Invoke-DbaQuery -SqlInstance $server -Database $Database -Query "DROP TABLE tempdb..$tempTableName" -EnableException

This comment has been minimized.

Copy link
@jaxnoth

jaxnoth Feb 12, 2020

Author Contributor

Added EnableException flag

This comment has been minimized.

Copy link
@potatoqualitee

potatoqualitee Feb 12, 2020

Member

awesomeeeee 💯


foreach ($sqlview in $InputObject) {
$Database = $sqlview.Parent.Name
$server = $sqlview.Parent.Parent

This comment has been minimized.

Copy link
@wsmelton

wsmelton Feb 12, 2020

Member

Is this not overwriting the object you created on line 263?

This comment has been minimized.

Copy link
@jaxnoth

jaxnoth Feb 12, 2020

Author Contributor

This is the same code that is in Copy-DBADbTableData. Would you like us to fix it in both scripts?

This comment has been minimized.

Copy link
@potatoqualitee

potatoqualitee Feb 12, 2020

Member

I'd like to get this merged 🌈, so if the answer is yes, it will need to be a new PR.

This comment has been minimized.

Copy link
@jaxnoth

jaxnoth Feb 13, 2020

Author Contributor

It isn't the same. This is reusing the server variable after putting any objects into the InputObject. Until the reuse causes an issue, I wouldn't bother with using a different variable name.

This comment has been minimized.

Copy link
@wsmelton

wsmelton Feb 13, 2020

Member

Not a good mindset to have with the size of userbase we have on this module. I will take care of it later today.

This comment has been minimized.

Copy link
@jaxnoth

jaxnoth Feb 13, 2020

Author Contributor

I'll adjust my mindset there. Still getting a handle on the expectations. Thanks.

try {
if ($Truncate -eq $true) {
if ($Pscmdlet.ShouldProcess($destServer, "Truncating table $fqtndest")) {
$null = $destServer.Databases[$DestinationDatabase].ExecuteNonQuery("TRUNCATE TABLE $fqtndest")

This comment has been minimized.

Copy link
@wsmelton

wsmelton Feb 12, 2020

Member

This is utilizing ExecuteNonQuery() but in other places Invoke-DbaQuery is used...is there any reason it is not consistent?

This comment has been minimized.

Copy link
@jaxnoth

jaxnoth Feb 12, 2020

Author Contributor

This is the same code that is in Copy-DBADbTableData. Would you like us to fix it in both scripts?

This comment has been minimized.

Copy link
@potatoqualitee

potatoqualitee Feb 12, 2020

Member

I'd like to get this merged 🌈, so if the answer is yes, it will need to be a new PR. (bout to copy/paste)

This comment has been minimized.

Copy link
@jaxnoth

jaxnoth Feb 13, 2020

Author Contributor

I'm updating this in PR #6358

Write-Progress -id 1 -activity "Inserting rows" -status "Complete" -Completed
}

$server.ConnectionContext.SqlConnectionObject.Close()

This comment has been minimized.

Copy link
@wsmelton

wsmelton Feb 12, 2020

Member

Do you also need to close out the Destination connection utilized in the command?

This comment has been minimized.

Copy link
@jaxnoth

jaxnoth Feb 12, 2020

Author Contributor

I believe the line under this "$bulkCopy.Close()" accomplishes that. It is the same code as in Copy-DBADbTableData

This comment has been minimized.

Copy link
@potatoqualitee

potatoqualitee Feb 12, 2020

Member

I'd like to get this merged 🌈, so if the answer is yes, since it matches copytable it will need to be a new PR.

@potatoqualitee

This comment has been minimized.

Copy link
Member

potatoqualitee commented Feb 12, 2020

Thank you both so much 🙇 Sorry for the messy merge, @wsmelton - I want to ensure @jaxnoth is counted as a contributor.

@potatoqualitee potatoqualitee merged commit ed695b1 into sqlcollaborative:development Feb 12, 2020
4 checks passed
4 checks passed
Module imports on all platforms (ubuntu-latest)
Details
Module imports on all platforms (windows-latest)
Details
Module imports on all platforms (macOS-latest)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@jaxnoth

This comment has been minimized.

Copy link
Contributor Author

jaxnoth commented Feb 12, 2020

Thanks for working with us! You may have closed the pull request a little early. I commented on the EnableException flag right before I pushed the final commit.

@wsmelton

This comment has been minimized.

Copy link
Member

wsmelton commented Feb 12, 2020

@jaxnoth just do a new PR to fix the additional items...

@potatoqualitee

This comment has been minimized.

Copy link
Member

potatoqualitee commented Feb 12, 2020

Oh lol whoops, already in the Gallery. Yes, new PR please. Thanks again for your time.

@jaxnoth jaxnoth mentioned this pull request Feb 12, 2020
0 of 10 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can’t perform that action at this time.