Is there an existing issue for this?
Description
Would it be worth changing SqlCommandSplitter to use https://github.com/microsoft/sqlmanagementobjects instead? It's used for SSMS and the like and is Microsoft-backed. The license is MIT, same as this repo, so there should be no issue with licensing.
It would remove the need to manage a SQL parser and batch splitter in this project.
Things to consider that I don't have a hard answer for:
- Does it matter that it's taking another dependency?
- Given
GO is not part of the SQL standard, should SqlCommandSplitter as it currently exists really be in dbup-core? Shouldn't the implementation that splits on GO be moved to dbup-sqlserver?
- Does there even need to be a
SqlCommandSplitter at all? Can we get away with just putting the splitting code directly in DbUp.SqlServer.SqlConnectionManager.SplitScriptIntoCommands? It works in there and there are no other real uses of SqlCommandSplitter that I can see (aside from tests - which wouldn't be needed once the parsing and splitting logic was handed off to another package).
I can make a PR for it if it would be wanted. I've done most of the work already (there are a couple of tests that fail when using SMO, but those tests are inaccurate when compared to actual SSMS behaviour; I'm going to make a new issue about those) so it wouldn't be that much effort to tidy it up for PR.
What is the impact?
Chiefly, lowered maintenance burden for dbup-sqlserver and more reliable batch splitting.
It could be considered a breaking change because SMO will error on invalid SQL (which some of the inconsistent tests I mentioned earlier contain), but then it could also be argued that those scripts should never be allowed through in the first place, split into separate batches or not.
It would also correctly send multiple copies of a batch if a script ever specifies a repeat number after the batch separator.
If that is not considered a breaking change, there should be no immediately-noticeable effects to users, though the batch splitting will become more reliable (aside from an issue I have open for SMO at the moment, but that is currently an outstanding issue here as well)
Is there an existing issue for this?
Description
Would it be worth changing
SqlCommandSplitterto use https://github.com/microsoft/sqlmanagementobjects instead? It's used for SSMS and the like and is Microsoft-backed. The license is MIT, same as this repo, so there should be no issue with licensing.It would remove the need to manage a SQL parser and batch splitter in this project.
Things to consider that I don't have a hard answer for:
GOis not part of the SQL standard, shouldSqlCommandSplitteras it currently exists really be in dbup-core? Shouldn't the implementation that splits onGObe moved to dbup-sqlserver?SqlCommandSplitterat all? Can we get away with just putting the splitting code directly inDbUp.SqlServer.SqlConnectionManager.SplitScriptIntoCommands? It works in there and there are no other real uses ofSqlCommandSplitterthat I can see (aside from tests - which wouldn't be needed once the parsing and splitting logic was handed off to another package).I can make a PR for it if it would be wanted. I've done most of the work already (there are a couple of tests that fail when using SMO, but those tests are inaccurate when compared to actual SSMS behaviour; I'm going to make a new issue about those) so it wouldn't be that much effort to tidy it up for PR.
What is the impact?
Chiefly, lowered maintenance burden for dbup-sqlserver and more reliable batch splitting.
It could be considered a breaking change because SMO will error on invalid SQL (which some of the inconsistent tests I mentioned earlier contain), but then it could also be argued that those scripts should never be allowed through in the first place, split into separate batches or not.
It would also correctly send multiple copies of a batch if a script ever specifies a repeat number after the batch separator.
If that is not considered a breaking change, there should be no immediately-noticeable effects to users, though the batch splitting will become more reliable (aside from an issue I have open for SMO at the moment, but that is currently an outstanding issue here as well)