pg/catalog: dropping a commented object emits COMMENT ... IS NULL after the DROP, so the migration fails to apply

Summary

When an object carrying a comment is dropped, GenerateMigration emits a COMMENT ON … IS NULL for it in PhaseMain, after the object's own DROP in PhasePre. PostgreSQL has no COMMENT … IF EXISTS, so the statement errors and the whole migration fails on apply.

This is not specific to one object kind — tables, views and procedures all reproduce.

Reproduction

Applied to postgres:16-alpine via the startPGContainer harness, seeding from, generating the plan, then running plan.SQL():

drop table with comment
  [0] phase=0 DROP TABLE "public"."t" CASCADE
  [1] phase=1 COMMENT ON TABLE "public"."t" IS NULL
  >>> APPLY FAILED: ERROR: relation "public.t" does not exist (SQLSTATE 42P01)

drop procedure with comment
  [0] phase=0 DROP PROCEDURE "public"."purge_rows"()
  [1] phase=1 COMMENT ON PROCEDURE public.purge_rows() IS NULL
  >>> APPLY FAILED: ERROR: procedure public.purge_rows() does not exist (SQLSTATE 42883)

drop view with comment
  [0] phase=0 DROP VIEW "public"."v"
  [1] phase=1 COMMENT ON VIEW "public"."v" IS NULL
  >>> APPLY FAILED: ERROR: relation "public.v" does not exist (SQLSTATE 42P01)

Fixtures:

-- table
CREATE TABLE t (id int); COMMENT ON TABLE t IS 'a table';           -- to: empty
-- procedure
CREATE PROCEDURE purge_rows() LANGUAGE plpgsql AS $$ BEGIN NULL; END; $$;
COMMENT ON PROCEDURE purge_rows() IS 'Purges old rows';             -- to: empty
-- view
CREATE TABLE b (id int); CREATE VIEW v AS SELECT id FROM b;
COMMENT ON VIEW v IS 'a view';                                      -- to: CREATE TABLE b (id int);

Cause

diffComments (pg/catalog/diff_comment.go) compares name-keyed comment maps. When an object is dropped its comment leaves toMap, which yields a DiffDrop entry indistinguishable from "the comment was removed but the object stayed". generateCommentDDL (pg/catalog/migration_comment.go) then renders every DiffDrop entry as COMMENT ON … IS NULL at PhaseMain:

case DiffDrop:
    sql := formatCommentSQL(from, entry.ObjType, entry.ObjDescription, entry.SubID, "")
    if sql != "" {
        ops = append(ops, MigrationOp{Type: OpComment, …, Phase: PhaseMain, …})
    }

A comment does not outlive its object, so the op is redundant as well as invalid: dropping the object already removes the comment.

Suggested fix

Suppress the DiffDrop comment op when the object it targets is absent from the to catalog. commentObjectTarget formats rather than resolves, so this needs a real existence check per ObjType — conservatively keeping the op for any type that cannot be resolved, so no currently-valid output changes.

Why it has not been caught

migration_roundtrip_test.go applies the plan with LoadSQL(from + migration) — omni's own executor accepts COMMENT ON for a missing object, so the round trip passes. The container tests apply against a real Postgres, but none of them drops a commented object.

Worth adding a container case that applies a plan dropping a commented object, since that is the check that would have failed.

Found via

https://github.com/bytebase/omni/pull/407, where a Codex review flagged the procedure case. The reproduction above extends it to tables and views.

🤖 Generated with Claude Code

Start with pg/catalog/diff_comment.go and pg/catalog/migration_comment.go, tracing diffComments, commentObjectTarget, and generateCommentDDL. Review migration_roundtrip_test.go and the startPGContainer harness, then add or run a real-PostgreSQL case that drops commented tables, views, and procedures. Done means applying the generated plan succeeds without COMMENT statements targeting dropped objects.