dataplat / Invoke-SqlCmd2

PowerShell module containing Invoke-SqlCmd2
MIT License
68 stars 35 forks source link

GO statements in SQL Scripts fail #10

Closed juancrl closed 6 years ago

juancrl commented 7 years ago

I have to execute many SQL scripts containing multiple batches inside (all separated with GO statement). I know this is not a part of T-SQL but they are accepted and run by Invoke-SQLCMD. However, Invoke-SQLCMD2 does not. The workaround is to remove these statements by script, but there are some cases where this breaks it (as when a CREATE VIEW is done, it must be in a separate batch). The second workaround would be splitting the scripts, but that would take a long time and generate more complexity to the project. So I had to sadly revert to old Invoke-SQLCMD. Is there any option to make Invoke-SQLCMD2 accept these files ? Thanks in advance...

RamblingCookieMonster commented 7 years ago

Hiyo!

I know this isn't always possible, but generally, I'd recommend (1) not writing queries with GO, and (2) modifying existing queries to not use GO.

If that's not possible, your best bet will be another command that supports GO.

Some folks have suggested splitting on GO and submitting multiple queries. Not a big fan of this. Outside of any functional differences, this would IMHO be impossible to reliably parse. I suppose we could throw in a -SplitOnGO that attempts this, with the assumption that it can and will break given certain query strings.... but I would personally advocate against that : )

Anyone else want to chime in? This isn't really my area of expertise!

Cheers!

niphlod commented 7 years ago

welll....techinically the "GO" in "sqlcmd mode" is only valid for lines starting with GO and nothing else but spaces (or tabs, etc). soooooo, apart from being able to handle different resultsets (which I'm not sure it's the proper case) and/or error management, splitting should be a breeze

[regex]::split($thestatement, '(?smi)^[\s]?GO[\s]?$')

alevyinroc commented 7 years ago

I believe @niphlod's suggestion to split on GO is what's used in the Install-DbaWhoIsActive command in dbatools to break up the batches and run them independently.

RamblingCookieMonster commented 7 years ago

Awesome! Didn't realize that was a restriction - if so, that solution makes sense to me : )

Might consider whether to add a switch to parse this, or to just make it clear that this happens in the comment based help - doing this by default won't be a breaking change, as this was never supported in the past.

Cheers!

baingar commented 6 years ago

Are there any plans to add support for GO statements?

RamblingCookieMonster commented 6 years ago

Hiyo!

So! Not a DBA, and I'll defer to DBAs, but given what @niphlod mentioned (GO only supported if it's the first and only word on a line), personally I think it would be safe and convenient to support this by default.

Cheers!

ConstantineK commented 6 years ago

I think its a bit harder than the regex you propose @niphlod, check out https://github.com/chucknorris/roundhouse/blob/master/db/SQLServer/TestRoundhousE/up/0003_TestBatchSplitter.sql

niphlod commented 6 years ago

There are definitely ways to avoid depending on SMO for "uber-parsing" (https://stackoverflow.com/questions/7690380/regular-expression-to-match-all-comments-in-a-t-sql-script/33947706#33947706) but I reallly think anything exported from dbatools or any other "sane" client would solve the 99% of the cases invoke-sqlcmd2 now "suffers". Let's face it, the real deal here is that despite the name, invoke-sqlcmd2 will never be 100% compatible with invoke-sqlcmd,sqlcmd,isql,osql weird syntaxes (irregardless of go, and nested comments, there is also :setvar: .... ). Can't we just help with the 99% and leave the weird 1% aside instead of refuse to help where we know for sure there is a good usercase scenarios ?

RamblingCookieMonster commented 6 years ago

Hiyo!

A reasonable compromise might be to include some form of GO parsing, but only if a -ParseGo or similar switch is indicated.

IANADBA, but my initial thoughts on implementation:

This way the command would continue to work as is, but folks who want to use it with queries involving GO can do so, with a built in option and minimal effort.

I'd defer to DBAs and other dbatool authors, take my thoughts with a grain of salt : )

Cheers!

niphlod commented 6 years ago

I'm all in favour of the previous points. Recapping a bit, so we can "close the deal"...

Yesterday with @ConstantineK we had a fun time testing the most crazy things.

Basically a little modification of the original to include yet another corner case, i.e. [regex]::split($thestatement, '(?smi)^[\s]*GO[\s]*$') works for all split statements, in the "perfect" way (i.e. there are no cornercase to advertise about possibly incorrect splits on GO).

This would leave basically only 2 open points on "SMO" vs "text-based" parsing:

For the original requirement of this issue, "supporting GO statements", the aforementioned solution is a win-win.

Setvar will probably never implemented, but for comments (which DO NOT consider nested ones) there's yet another simple and fast regex to the rescue.

Implementing BOTH regexes (comments parsing and GO splitting) will make invoke-sqlcmd2 on par of SMO/invoke-sqlcmd/sqlcmd/isql/osql except for:

If you're all on board with it, I'll make a PR and you can throw at it everything you can think of to test we aren't introducing bugs.

TheRockStarDBA commented 6 years ago

Please fix this as it will be incredibly useful for script deployments.

aravindkumarh23 commented 4 years ago

@niphlod this doesnt seem to work I tried using -parsego swtich and it throwed me error saying "Invoke-Sqlcmd2 : A parameter cannot be found that matches parameter name 'ParseGo'."

below are commands i used. please help.

invoke-sqlcmd2 -ServerInstance $srv -Credential $pscredential -Database master -Query $q1 -ParseGo true invoke-sqlcmd2 -ServerInstance $srv -Credential $pscredential -Database master -Query $q1 -ParseGo