migrate icon indicating copy to clipboard operation
migrate copied to clipboard

Allow multi-statement mode to be enabled per-migration in Postgres

Open ndisidore opened this issue 3 years ago • 3 comments

overview.

The multi-statement mode work for Postgres is fantastic and addresses a long-standing issue we've had with this. By design, the multi-statement mode parser is meant to be pretty basic. It works well in the base case, but unfortunately it does break when doing things like function and stored procedures CREATE/UPDATES, as lines in these are expected to terminate with semicolons

An example of a problematic migration:

-- create function
CREATE OR REPLACE FUNCTION update_modified()
RETURNS TRIGGER AS $$
BEGIN
    NEW.modified = now();
    RETURN NEW;
END;
$$ language 'plpgsql';

The goal of this PR is to allow per-migration enablement of multi-statement mode (while still allowing global enablement if necessary) via a flag within the migration itself. This is admittedly still a relatively naive approach, as it is for the full file

Inspiration for this was gleaned from sql-migrate, which we can see attempts to address the problem in a similar manner https://github.com/rubenv/sql-migrate/blob/f842348935589e4563be545226d465178bd439cf/sqlparse/sqlparse.go#L106

ndisidore avatar Feb 01 '22 00:02 ndisidore

Pull Request Test Coverage Report for Build 1775594640

  • 36 of 44 (81.82%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 57.955%

Changes Missing Coverage Covered Lines Changed/Added Lines %
database/postgres/postgres.go 36 44 81.82%
<!-- Total: 36 44
Totals Coverage Status
Change from base Build 1689206270: 0.2%
Covered Lines: 3763
Relevant Lines: 6493

💛 - Coveralls

coveralls avatar Feb 01 '22 00:02 coveralls

Would take care of https://github.com/golang-migrate/migrate/issues/16

andream16 avatar Feb 01 '22 09:02 andream16

#691 is another great PR that, I believe, attempts to address a different (but related) problem where the contents of comments themselves (e.g. if a comment as a ; in it) can impact the multi-statement parser - unfortunately this problem would still exist

I'm open to alternatives if we don't want comments to have side affects, but in lieu of any I'd encourage you to re-evaluate that policy, as its widely used by many popular migration frameworks (goose, sql-migrate, ...) to good effect

ndisidore avatar Feb 16 '22 03:02 ndisidore