diff --git a/pg/catalog/migration_comment_test.go b/pg/catalog/migration_comment_test.go index 99eeb4ad..159747cc 100644 --- a/pg/catalog/migration_comment_test.go +++ b/pg/catalog/migration_comment_test.go @@ -271,6 +271,75 @@ func TestMigrationComment(t *testing.T) { } }) + t.Run("single quotes in a comment are doubled", func(t *testing.T) { + fromSQL := `CREATE TABLE t (id int);` + toSQL := ` + CREATE TABLE t (id int); + COMMENT ON TABLE t IS 'User''s table'; + ` + from, err := LoadSQL(fromSQL) + if err != nil { + t.Fatal(err) + } + to, err := LoadSQL(toSQL) + if err != nil { + t.Fatal(err) + } + diff := Diff(from, to) + plan := GenerateMigration(from, to, diff) + ops := plan.Filter(func(op MigrationOp) bool { + return op.Type == OpComment + }).Ops + if len(ops) != 1 { + t.Fatalf("expected 1 Comment op, got %d; ops: %v", len(ops), opsSQL(plan)) + } + // The catalog holds the comment unescaped. Rendering it back with a bare + // quote closes the literal early and the statement no longer parses, so + // the whole literal has to be checked, not just the text around it. + if !strings.Contains(ops[0].SQL, `IS 'User''s table'`) { + t.Errorf("expected the quote doubled in the emitted literal, got: %s", ops[0].SQL) + } + }) + + t.Run("dropping a procedure leaves a same-named function's comment alone", func(t *testing.T) { + // Procedures and functions share one catalog, and these two share a name + // as well — only the argument list separates them. That is the shape a + // comment differ keyed on the bare name gets wrong: it reads the dropped + // procedure's comment as a change to the surviving function. + const survivor = ` + CREATE FUNCTION routine_x() RETURNS void + LANGUAGE plpgsql AS $$ BEGIN NULL; END; $$; + COMMENT ON FUNCTION routine_x() IS 'The function'; + ` + fromSQL := survivor + ` + CREATE PROCEDURE routine_x(n integer) LANGUAGE plpgsql AS $$ BEGIN NULL; END; $$; + COMMENT ON PROCEDURE routine_x(integer) IS 'The procedure'; + ` + toSQL := survivor + from, err := LoadSQL(fromSQL) + if err != nil { + t.Fatal(err) + } + to, err := LoadSQL(toSQL) + if err != nil { + t.Fatal(err) + } + diff := Diff(from, to) + plan := GenerateMigration(from, to, diff) + if ops := filterOps(plan, OpDropFunction); len(ops) != 1 { + t.Fatalf("expected 1 DropFunction op for the procedure, got %d; ops: %v", len(ops), opsSQL(plan)) + } + // routine_x() is the survivor, routine_x(integer) the one being dropped. + // The plan still clears the dropped routine's comment, which is issue + // #408 and not what this case is about; matching on the full signature + // keeps the two apart. + for _, op := range plan.Ops { + if op.Type == OpComment && strings.Contains(op.SQL, "routine_x()") { + t.Errorf("the surviving function's comment must not change: %s", op.SQL) + } + } + }) + t.Run("COMMENT ON PROCEDURE uses PROCEDURE not FUNCTION", func(t *testing.T) { fromSQL := ` CREATE PROCEDURE do_work(x integer) diff --git a/pg/catalog/migration_extension_test.go b/pg/catalog/migration_extension_test.go index e995cb8e..402d8d54 100644 --- a/pg/catalog/migration_extension_test.go +++ b/pg/catalog/migration_extension_test.go @@ -86,6 +86,27 @@ func TestMigrationExtension(t *testing.T) { } }, }, + { + name: "Extension name does not become a schema", + fromSQL: "", + toSQL: "CREATE EXTENSION test_ext; CREATE TABLE t1 (id int);", + check: func(t *testing.T, plan *MigrationPlan) { + // CREATE EXTENSION names an extension, not a schema. Registering + // one under its own name would emit a CREATE SCHEMA that fails on + // any database where the extension is already installed. + extOps := plan.Filter(func(op MigrationOp) bool { + return op.Type == OpCreateExtension + }) + if len(extOps.Ops) != 1 { + t.Fatalf("expected 1 CreateExtension op, got %d", len(extOps.Ops)) + } + for _, op := range plan.Ops { + if op.Type == OpCreateSchema { + t.Errorf("extension must not produce a schema, got: %s", op.SQL) + } + } + }, + }, { name: "Extension operations ordered before types and tables", fromSQL: "", diff --git a/pg/catalog/migration_index_test.go b/pg/catalog/migration_index_test.go index 0ab1b0a9..a16344f0 100644 --- a/pg/catalog/migration_index_test.go +++ b/pg/catalog/migration_index_test.go @@ -191,6 +191,27 @@ CREATE INDEX idx_t_name ON t (name, email);`, } }, }, + { + name: "EXCLUDE backing index NOT generated", + fromSQL: `CREATE TABLE t (id int, r int4range);`, + toSQL: `CREATE TABLE t (id int, r int4range, CONSTRAINT t_r_excl EXCLUDE USING gist (r WITH &&));`, + check: func(t *testing.T, plan *MigrationPlan) { + // EXCLUDE carries its own index the way PK and UNIQUE do, so the + // ADD CONSTRAINT is the whole change. A CREATE INDEX beside it + // would fail on apply: the name is already taken by the + // constraint's index. + if ops := filterOps(plan, OpCreateIndex); len(ops) != 0 { + t.Errorf("expected no CreateIndex op for EXCLUDE backing index, got %d: %s", len(ops), ops[0].SQL) + } + ops := filterOps(plan, OpAddConstraint) + if len(ops) != 1 { + t.Fatalf("expected 1 AddConstraint op, got %d", len(ops)) + } + if !strings.Contains(ops[0].SQL, "EXCLUDE") { + t.Errorf("expected EXCLUDE in constraint DDL, got %s", ops[0].SQL) + } + }, + }, { name: "PK/UNIQUE backing indexes NOT generated", fromSQL: `CREATE TABLE t (id int);`,