Support MSSQL batch statements (Resolves #652)
The sql query gets split to batches with https://github.com/denisenkom/go-mssqldb/blob/master/batch/batch.go Resolves #652 and #466
Pull Request Test Coverage Report for Build 2245625366
- 18 of 23 (78.26%) changed or added relevant lines in 1 file are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage increased (+0.05%) to 57.861%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| database/sqlserver/sqlserver.go | 18 | 23 | 78.26% |
| <!-- | Total: | 18 | 23 |
| Totals | |
|---|---|
| Change from base Build 1997075650: | 0.05% |
| Covered Lines: | 3754 |
| Relevant Lines: | 6488 |
π - Coveralls
@dhui thanks for taking a look at this. I can definitely look into those flags and at least confirm that the SQL Server tests pass, though like in #714 I probably won't tackle unrelated failures with other databases. Hopefully next week!
Thanks, @dhui. I'll look into those flags and add something by the next week
@glebteterin heads up, I suspect that the changes merged in #714 will help you get passing sqlserver tests - they were broken for me prior to those changes, so be sure to pull them in!
@jfhbrook-at-work can this be merged any time soon? I'm currently struggling with a migration that contains a couple of functions that can only be deployed to the database schema as a single batch statement.
@tmeckel My team decided to make an internal fork and float this patch, rather than working to push it across the finish line. I can tell you, though, that this patch works fine for us - tests included.
@jfhbrook-at-work thanks for the quick answer! So, for me it seems I either have to build a version on my own with this patch included or I wait for the team to implement this functionality aside this PR. If so, is there a time frame in which the functionality will be provided in an official build?
I would say "when someone addresses the open issues in the PR". That's not gonna be me today - perhaps it will be you?
@jfhbrook-at-work Okay, so the open issues are the ones @dhui mentioned in https://github.com/golang-migrate/migrate/pull/666#pullrequestreview-913412810?
That's my understanding, yes.
@jfhbrook-at-work I just had a quick look how the stuff is implemented in the Postgres driver. Very straight forward. But I don't want to simply "hijack" the PR.
@glebteterin will you have any time soon to implement the changes requested by @dhui
The required changes are done, but I'm getting some flaky errors. @dhui any ideas what could it be?
@glebteterin seems to me that the GitHub build agent wasn't able to start the database containers correctly. When I look over the build logs every single database test failed because the TCP connections couldn't be established. @jfhbrook-at-work can you restart the CI build? Perhaps this error was transient.
@Fontinalis YEAH πΊπ» can't wait to get this merged ππΌ π
@tmeckel Gonna check it one last time and merge it either this weekend or Monday. Had some last minute things coming up..
@tmeckel Gonna check it one last time and merge it either this weekend or Monday. Had some last minute things coming up..
@Fontinalis will this PR merged soon?
FYI, this PR will need to be updated after #758 is merged
Hello, Thanks for this PR I am also encountering this issue and have a need to support MSSQL batch statements in our migrate sql scripts. I see the https://github.com/golang-migrate/migrate/pull/758 is now merged. @dhui @Fontinalis any chances of this PR also getting merged anytime soon?
Hi, Could you please finally merge it?
Hello, Could anyone please merge this PR ?
@dhui would it be possible to merge this if someone addresses any remaining PR comments?
We've been using it in a forked version for a year with no issues, but it would sure be nice if we could stop having to use/maintain our fork and just use the upstream : )
Cheers!
Ping @dhui, is there anything I can do to help get this PR merged in?
We've tested it for a year now and it works fine.
Thanks!!