Support for merging SQLCMD scripts

[jmezach]

In version 1.1.0 we've added support for pre- and post deployment scripts (see #9). Unfortunately we do not yet support including other scripts from those scripts using the :r OtherScript.sql syntax.

Some initial work has been done to support this (see feature/sqlcmd-file-merging branch), but it has some limitations in that it doesn't work cross-platform. This is due to the fact that the SQL libraries we're currently using seems to uppercase the path, which doesn't work on platforms that have a case-sensitive file system (ie. Linux). Ideally we would like to keep this library cross-platform, so we'll need to figure out how to work around this.

[ltk-mpadelsky]

Is this all that is holding up merging in https://github.com/jmezach/MSBuild.Sdk.SqlProj/tree/feature/sqlcmd-file-merging ? I'm wondering if you could just release this knowing that reference path resolution within DacFX is not working cross-platform yet (or I should say, converting given the platform its building and resolving on).

I notice the DACExtensions project you commented on hasnt been worked on for about 3 years, which leaves me little hope that they'll resolve this anytime soon.

I'm wondering if a short-term solution would be to do something like this.

• Include 2 post/pre scripts, and ONLY include reference paths in them
• 1 set of pre/post scripts is named .windows. and the other .linux. with proper pathing syntax in both.
• developers could build for their single environment, or then use csproject conditionals to determine which one should be the included pre/post scripts

Does that make sense? I haven't tried proving this out on my side yet to know if that's a viable solution or not.

I just know as it stands now, this SDK doesn't support resolving references at all (in v 1.1.0), so it would be nice to have SOMETHING sooner than later.

[ErikEJ]

1.1.0 DOES support pre and post scripts, what makes you think otherwise? They must just be single scripts, and reside in the correctly named folders

[ltk-mpadelsky]

1.1.0 DOES support pre and post scripts, what makes you think otherwise? They must just be single scripts, and reside in the correctly named folders

Sorry, @ErikEJ ! I should've proof read my response a little better. I edited it above. I meant to say that it doesn't support resolving the references in those scripts!

[jmezach]

I was thinking about making this work in a way that it would work as expected on Windows, while having the "handicap" on Linux and macOS of having to use upper-cased filenames. That way it would pass CI and it would just work on Windows as expected. Not sure when I'll have the time to finish this though.

[jmezach]

I've had another look at this, but I don't think there's an easy fix for this issue. Not only does it uppercase the filename, but it uppercases the absolute file path. That means I would need to rename the entire folder structure to be uppercases in order for it to work during CI, but I can't do that because some higher level folders are outside of my control.

So I guess we can only hope that someone at Microsoft is going to fix this, or we have to do the parsing ourselves which is non-trivial IMHO.

I found this which seems to provide the functionality I need to make this work, but so far I haven't found a way to use that from this project but I'll keep digging.

[jmezach]

I had another look at this. Apparently we're allowed to use code from microsoft/sqltoolsservice as long as we include the license. I've added a submodule and reference the ManagedBatchParser project directly and I think that will give us what we need to implement this. I'm already able to get the list of included scripts from the pre/post-deployment script. I think it will work for nested includes as well. Then I'll just need to figure out how to flatten everything into a single script. I can probably draw some inspiration from how SSDT does it ;).

[MarkIannucci]

@jmezach , I have a database project which utilizes the :r sqlcmd syntax in the post deployment context. I'm happy to help test when you get to that point (I'm afraid it would be limited to Windows though :-\ ).

[knoxi]

@jmezach some news when the merge SQL scripts support will be available? This one is currently blocking us from upgrading our solutions to container builds and it would be great to have this feature available.

[jeffrosenberg]

If nobody else is looking into this, I may attempt it in the near future, as it's also important for my organization. I can try to work on it next week.

[jmezach]

This isn't very high on our priority list since we are not using this in our projects. I did some initial work on it in feature/sqlcmd-file-merging which wasn't very successful in that it wouldn't work in cross-platform scenarios. I then tried again in feature/sqlcmd-merging-v2 by re-using code from another project (using a git submodule). That looked a bit more promising in that I was able to collect the list of files that would need merging. I just haven't gotten around to do the actual merging yet.

[ssa3512]

This is definitely a big blocker for my org as we use includes heavily.

[jeffrosenberg]

Quick status update: This is in progress; I hope to have a PR for review this week or early next.

[jmezach]

That's great news @jeffrosenberg. If you need any help with this please let me know.

[jmezach]

Support for this got added in 1.8.0 thanks to @jeffrosenberg.

Pinging @gregj77, @mrozwod, @ltk-mpadelsky, @MarkIannucci, @knoxi, @ssa3512 and @whighsmith since they seem to have been waiting for this feature to drop. Please let us know if this covers your scenario's and if it doesn't don't hesitate to file an issue.

[jeffrosenberg]

@jmezach looks like 1.8.0 actually got cut just before my commit, so this change isn't in there 😕

[jmezach]

Hmm, then something must have gone wrong. I'll have a look.

[jmezach]

You're right. I guess my local master branch wasn't up-to-date when I prepared the release. Pretty sure I pulled all the changes but I guess I was too soon or something. Anyway, I've cherry-picked the commit to the release branch, bumped the version number and the CI is running now. 1.8.1 that includes these changes should be up on NuGet momentarily.