Skip to content

Move SQL Server script splitting into dbup-sqlserver and use SMO for splitting instead of local implementation #22

@Pilchard123

Description

@Pilchard123

Is there an existing issue for this?

  • I have searched the existing open and closed issues

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)

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions