From b0e710c19dae692d55000d919883de0e9291da2d Mon Sep 17 00:00:00 2001 From: tianzhou Date: Mon, 6 Jul 2026 19:50:55 -0700 Subject: [PATCH 1/2] fix: defer new table inline FK when it depends on a new unique constraint (#506) When a new table's inline FK references columns on a pre-existing table that is gaining a UNIQUE or PK constraint (or unique index) in the same migration, the CREATE TABLE was emitted before the constraint it depends on, causing SQLSTATE 42830. Extend planFKRecreationForReplacedConstraints to also detect newly-added UNIQUE/PK constraints and unique indexes on modified tables, suppressing the FK from inline CREATE TABLE and deferring it to after the modify phase. Co-Authored-By: Claude Opus 4.6 --- internal/diff/table.go | 58 +++++++++++++++++-- .../diff.sql | 12 ++++ .../new.sql | 12 ++++ .../old.sql | 4 ++ .../plan.json | 32 ++++++++++ .../plan.sql | 12 ++++ .../plan.txt | 26 +++++++++ 7 files changed, 151 insertions(+), 5 deletions(-) create mode 100644 testdata/diff/create_table/issue_506_fk_depends_on_new_unique/diff.sql create mode 100644 testdata/diff/create_table/issue_506_fk_depends_on_new_unique/new.sql create mode 100644 testdata/diff/create_table/issue_506_fk_depends_on_new_unique/old.sql create mode 100644 testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.json create mode 100644 testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.sql create mode 100644 testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.txt diff --git a/internal/diff/table.go b/internal/diff/table.go index b85c24ee..cb9de679 100644 --- a/internal/diff/table.go +++ b/internal/diff/table.go @@ -620,7 +620,27 @@ func planFKRecreationForReplacedConstraints(modifiedTables []*tableDiff, addedTa } } } - if len(replaced) == 0 { + + // Unique/PK constraints and unique indexes newly added to existing tables + // in this migration. These are emitted in the modify phase, so FKs in new + // tables that depend on them must be deferred past CREATE TABLE (issue #506). + addedConstraints := make(map[string][]*ir.Constraint) + addedUniqueIndexes := make(map[string][]*ir.Index) + for _, td := range modifiedTables { + key := td.Table.Schema + "." + td.Table.Name + for _, c := range td.AddedConstraints { + if c.Type == ir.ConstraintTypeUnique || c.Type == ir.ConstraintTypePrimaryKey { + addedConstraints[key] = append(addedConstraints[key], c) + } + } + for _, idx := range td.AddedIndexes { + if idx.Type == ir.IndexTypeUnique { + addedUniqueIndexes[key] = append(addedUniqueIndexes[key], idx) + } + } + } + + if len(replaced) == 0 && len(addedConstraints) == 0 && len(addedUniqueIndexes) == 0 { return nil, nil, nil } @@ -654,9 +674,9 @@ func planFKRecreationForReplacedConstraints(modifiedTables []*tableDiff, addedTa } } - // FKs on newly added tables that target a replaced constraint: keep them - // out of CREATE TABLE (create phase runs before the modify-phase swap) and - // add them with the other recreated FKs instead. + // FKs on newly added tables that target a replaced or newly-added constraint + // (or unique index): keep them out of CREATE TABLE (create phase runs before + // the modify phase) and add them after the modify phase instead (#439, #506). suppressedInlineFKs = make(map[string]bool) for _, table := range addedTables { for _, name := range sortedKeys(table.Constraints) { @@ -664,7 +684,10 @@ func planFKRecreationForReplacedConstraints(modifiedTables []*tableDiff, addedTa if fk.Type != ir.ConstraintTypeForeignKey { continue } - if !fkReferencesAnyConstraint(fk, replaced[fkReferencedTableKey(fk)]) { + refKey := fkReferencedTableKey(fk) + if !fkReferencesAnyConstraint(fk, replaced[refKey]) && + !fkReferencesAnyConstraint(fk, addedConstraints[refKey]) && + !fkReferencesAnyUniqueIndex(fk, addedUniqueIndexes[refKey]) { continue } suppressedInlineFKs[constraintPathKey(fk)] = true @@ -720,6 +743,31 @@ func fkReferencesAnyConstraint(fk *ir.Constraint, candidates []*ir.Constraint) b return false } +// fkReferencesAnyUniqueIndex reports whether the FK's referenced columns match +// the column set of any of the given unique indexes. +func fkReferencesAnyUniqueIndex(fk *ir.Constraint, candidates []*ir.Index) bool { + refCols := make(map[string]bool, len(fk.ReferencedColumns)) + for _, col := range fk.ReferencedColumns { + refCols[col.Name] = true + } + for _, idx := range candidates { + if len(idx.Columns) != len(refCols) { + continue + } + matched := true + for _, col := range idx.Columns { + if !refCols[col.Name] { + matched = false + break + } + } + if matched { + return true + } + } + return false +} + // generateDropRecreatedFKsSQL drops foreign keys bound to unique/PK constraints // being replaced. Must run before the table modifications. (#439) func generateDropRecreatedFKsSQL(fks []*ir.Constraint, targetSchema string, collector *diffCollector) { diff --git a/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/diff.sql b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/diff.sql new file mode 100644 index 00000000..bf018f85 --- /dev/null +++ b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/diff.sql @@ -0,0 +1,12 @@ +CREATE TABLE IF NOT EXISTS child ( + id uuid, + parent_id uuid, + tenant text NOT NULL, + CONSTRAINT child_pkey PRIMARY KEY (id) +); + +ALTER TABLE parent +ADD CONSTRAINT parent_id_tenant_key UNIQUE (id, tenant); + +ALTER TABLE child +ADD CONSTRAINT child_parent_id_tenant_fkey FOREIGN KEY (parent_id, tenant) REFERENCES parent (id, tenant); diff --git a/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/new.sql b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/new.sql new file mode 100644 index 00000000..2e3c2f37 --- /dev/null +++ b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/new.sql @@ -0,0 +1,12 @@ +CREATE TABLE parent ( + id uuid PRIMARY KEY, + tenant text NOT NULL, + UNIQUE (id, tenant) +); + +CREATE TABLE child ( + id uuid PRIMARY KEY, + parent_id uuid, + tenant text NOT NULL, + FOREIGN KEY (parent_id, tenant) REFERENCES parent (id, tenant) +); diff --git a/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/old.sql b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/old.sql new file mode 100644 index 00000000..8c120b40 --- /dev/null +++ b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/old.sql @@ -0,0 +1,4 @@ +CREATE TABLE parent ( + id uuid PRIMARY KEY, + tenant text NOT NULL +); diff --git a/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.json b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.json new file mode 100644 index 00000000..28b4ab8a --- /dev/null +++ b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.json @@ -0,0 +1,32 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.12.0", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "312e14d55791468f6809e0d7b91325d6b8fd802ac0fd3fdfe2bad8e973e591a5" + }, + "groups": [ + { + "steps": [ + { + "sql": "CREATE TABLE IF NOT EXISTS child (\n id uuid,\n parent_id uuid,\n tenant text NOT NULL,\n CONSTRAINT child_pkey PRIMARY KEY (id)\n);", + "type": "table", + "operation": "create", + "path": "public.child" + }, + { + "sql": "ALTER TABLE parent\nADD CONSTRAINT parent_id_tenant_key UNIQUE (id, tenant);", + "type": "table.constraint", + "operation": "create", + "path": "public.parent.parent_id_tenant_key" + }, + { + "sql": "ALTER TABLE child\nADD CONSTRAINT child_parent_id_tenant_fkey FOREIGN KEY (parent_id, tenant) REFERENCES parent (id, tenant);", + "type": "table.constraint", + "operation": "create", + "path": "public.child.child_parent_id_tenant_fkey" + } + ] + } + ] +} diff --git a/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.sql b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.sql new file mode 100644 index 00000000..bf018f85 --- /dev/null +++ b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.sql @@ -0,0 +1,12 @@ +CREATE TABLE IF NOT EXISTS child ( + id uuid, + parent_id uuid, + tenant text NOT NULL, + CONSTRAINT child_pkey PRIMARY KEY (id) +); + +ALTER TABLE parent +ADD CONSTRAINT parent_id_tenant_key UNIQUE (id, tenant); + +ALTER TABLE child +ADD CONSTRAINT child_parent_id_tenant_fkey FOREIGN KEY (parent_id, tenant) REFERENCES parent (id, tenant); diff --git a/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.txt b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.txt new file mode 100644 index 00000000..d0c8aea1 --- /dev/null +++ b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.txt @@ -0,0 +1,26 @@ +Plan: 1 to add, 1 to modify. + +Summary by type: + tables: 1 to add, 1 to modify + +Tables: + + child + + child_parent_id_tenant_fkey (constraint) + ~ parent + + parent_id_tenant_key (constraint) + +DDL to be executed: +-------------------------------------------------- + +CREATE TABLE IF NOT EXISTS child ( + id uuid, + parent_id uuid, + tenant text NOT NULL, + CONSTRAINT child_pkey PRIMARY KEY (id) +); + +ALTER TABLE parent +ADD CONSTRAINT parent_id_tenant_key UNIQUE (id, tenant); + +ALTER TABLE child +ADD CONSTRAINT child_parent_id_tenant_fkey FOREIGN KEY (parent_id, tenant) REFERENCES parent (id, tenant); From 6edda0799e8c502b24412918171cf75892050309 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Mon, 6 Jul 2026 21:04:30 -0700 Subject: [PATCH 2/2] fix: filter partial indexes and add unique-index test variant (#506) Address Greptile/Copilot review feedback: - Filter out partial indexes (IsPartial) from addedUniqueIndexes - Expand test fixture to cover both variants: UNIQUE constraint and CREATE UNIQUE INDEX Co-Authored-By: Claude Opus 4.6 --- internal/diff/table.go | 2 +- .../diff.sql | 12 +++++ .../new.sql | 14 ++++++ .../old.sql | 5 +++ .../plan.json | 44 ++++++++++++++++++- .../plan.sql | 24 ++++++++++ .../plan.txt | 36 ++++++++++++++- 7 files changed, 132 insertions(+), 5 deletions(-) diff --git a/internal/diff/table.go b/internal/diff/table.go index cb9de679..405cf855 100644 --- a/internal/diff/table.go +++ b/internal/diff/table.go @@ -634,7 +634,7 @@ func planFKRecreationForReplacedConstraints(modifiedTables []*tableDiff, addedTa } } for _, idx := range td.AddedIndexes { - if idx.Type == ir.IndexTypeUnique { + if idx.Type == ir.IndexTypeUnique && !idx.IsPartial { addedUniqueIndexes[key] = append(addedUniqueIndexes[key], idx) } } diff --git a/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/diff.sql b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/diff.sql index bf018f85..5f021e1d 100644 --- a/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/diff.sql +++ b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/diff.sql @@ -5,8 +5,20 @@ CREATE TABLE IF NOT EXISTS child ( CONSTRAINT child_pkey PRIMARY KEY (id) ); +CREATE TABLE IF NOT EXISTS child2 ( + id uuid, + parent2_id uuid, + tenant text NOT NULL, + CONSTRAINT child2_pkey PRIMARY KEY (id) +); + ALTER TABLE parent ADD CONSTRAINT parent_id_tenant_key UNIQUE (id, tenant); +CREATE UNIQUE INDEX IF NOT EXISTS parent2_id_tenant_key ON parent2 (id, tenant); + ALTER TABLE child ADD CONSTRAINT child_parent_id_tenant_fkey FOREIGN KEY (parent_id, tenant) REFERENCES parent (id, tenant); + +ALTER TABLE child2 +ADD CONSTRAINT child2_parent2_id_tenant_fkey FOREIGN KEY (parent2_id, tenant) REFERENCES parent2 (id, tenant); diff --git a/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/new.sql b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/new.sql index 2e3c2f37..fd67ad72 100644 --- a/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/new.sql +++ b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/new.sql @@ -10,3 +10,17 @@ CREATE TABLE child ( tenant text NOT NULL, FOREIGN KEY (parent_id, tenant) REFERENCES parent (id, tenant) ); + +CREATE TABLE parent2 ( + id uuid PRIMARY KEY, + tenant text NOT NULL +); + +CREATE UNIQUE INDEX parent2_id_tenant_key ON parent2 (id, tenant); + +CREATE TABLE child2 ( + id uuid PRIMARY KEY, + parent2_id uuid, + tenant text NOT NULL, + FOREIGN KEY (parent2_id, tenant) REFERENCES parent2 (id, tenant) +); diff --git a/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/old.sql b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/old.sql index 8c120b40..49471fa4 100644 --- a/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/old.sql +++ b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/old.sql @@ -2,3 +2,8 @@ CREATE TABLE parent ( id uuid PRIMARY KEY, tenant text NOT NULL ); + +CREATE TABLE parent2 ( + id uuid PRIMARY KEY, + tenant text NOT NULL +); diff --git a/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.json b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.json index 28b4ab8a..8010efe3 100644 --- a/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.json +++ b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.json @@ -3,7 +3,7 @@ "pgschema_version": "1.12.0", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { - "hash": "312e14d55791468f6809e0d7b91325d6b8fd802ac0fd3fdfe2bad8e973e591a5" + "hash": "a2e9edaed4b13aa629d2ed847ffd01634bf02251aaa187e810dc64ecc17497e1" }, "groups": [ { @@ -14,17 +14,57 @@ "operation": "create", "path": "public.child" }, + { + "sql": "CREATE TABLE IF NOT EXISTS child2 (\n id uuid,\n parent2_id uuid,\n tenant text NOT NULL,\n CONSTRAINT child2_pkey PRIMARY KEY (id)\n);", + "type": "table", + "operation": "create", + "path": "public.child2" + }, { "sql": "ALTER TABLE parent\nADD CONSTRAINT parent_id_tenant_key UNIQUE (id, tenant);", "type": "table.constraint", "operation": "create", "path": "public.parent.parent_id_tenant_key" - }, + } + ] + }, + { + "steps": [ + { + "sql": "CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS parent2_id_tenant_key ON parent2 (id, tenant);", + "type": "table.index", + "operation": "create", + "path": "public.parent2.parent2_id_tenant_key" + } + ] + }, + { + "steps": [ + { + "sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'parent2_id_tenant_key';", + "directive": { + "type": "wait", + "message": "Creating index parent2_id_tenant_key" + }, + "type": "table.index", + "operation": "create", + "path": "public.parent2.parent2_id_tenant_key" + } + ] + }, + { + "steps": [ { "sql": "ALTER TABLE child\nADD CONSTRAINT child_parent_id_tenant_fkey FOREIGN KEY (parent_id, tenant) REFERENCES parent (id, tenant);", "type": "table.constraint", "operation": "create", "path": "public.child.child_parent_id_tenant_fkey" + }, + { + "sql": "ALTER TABLE child2\nADD CONSTRAINT child2_parent2_id_tenant_fkey FOREIGN KEY (parent2_id, tenant) REFERENCES parent2 (id, tenant);", + "type": "table.constraint", + "operation": "create", + "path": "public.child2.child2_parent2_id_tenant_fkey" } ] } diff --git a/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.sql b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.sql index bf018f85..f1619ce5 100644 --- a/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.sql +++ b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.sql @@ -5,8 +5,32 @@ CREATE TABLE IF NOT EXISTS child ( CONSTRAINT child_pkey PRIMARY KEY (id) ); +CREATE TABLE IF NOT EXISTS child2 ( + id uuid, + parent2_id uuid, + tenant text NOT NULL, + CONSTRAINT child2_pkey PRIMARY KEY (id) +); + ALTER TABLE parent ADD CONSTRAINT parent_id_tenant_key UNIQUE (id, tenant); +CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS parent2_id_tenant_key ON parent2 (id, tenant); + +-- pgschema:wait +SELECT + COALESCE(i.indisvalid, false) as done, + CASE + WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total + ELSE 0 + END as progress +FROM pg_class c +LEFT JOIN pg_index i ON c.oid = i.indexrelid +LEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid +WHERE c.relname = 'parent2_id_tenant_key'; + ALTER TABLE child ADD CONSTRAINT child_parent_id_tenant_fkey FOREIGN KEY (parent_id, tenant) REFERENCES parent (id, tenant); + +ALTER TABLE child2 +ADD CONSTRAINT child2_parent2_id_tenant_fkey FOREIGN KEY (parent2_id, tenant) REFERENCES parent2 (id, tenant); diff --git a/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.txt b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.txt index d0c8aea1..50fd88ce 100644 --- a/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.txt +++ b/testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.txt @@ -1,17 +1,22 @@ -Plan: 1 to add, 1 to modify. +Plan: 2 to add, 2 to modify. Summary by type: - tables: 1 to add, 1 to modify + tables: 2 to add, 2 to modify Tables: + child + child_parent_id_tenant_fkey (constraint) + + child2 + + child2_parent2_id_tenant_fkey (constraint) ~ parent + parent_id_tenant_key (constraint) + ~ parent2 + + parent2_id_tenant_key (index) DDL to be executed: -------------------------------------------------- +-- Transaction Group #1 CREATE TABLE IF NOT EXISTS child ( id uuid, parent_id uuid, @@ -19,8 +24,35 @@ CREATE TABLE IF NOT EXISTS child ( CONSTRAINT child_pkey PRIMARY KEY (id) ); +CREATE TABLE IF NOT EXISTS child2 ( + id uuid, + parent2_id uuid, + tenant text NOT NULL, + CONSTRAINT child2_pkey PRIMARY KEY (id) +); + ALTER TABLE parent ADD CONSTRAINT parent_id_tenant_key UNIQUE (id, tenant); +-- Transaction Group #2 +CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS parent2_id_tenant_key ON parent2 (id, tenant); + +-- Transaction Group #3 +-- pgschema:wait +SELECT + COALESCE(i.indisvalid, false) as done, + CASE + WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total + ELSE 0 + END as progress +FROM pg_class c +LEFT JOIN pg_index i ON c.oid = i.indexrelid +LEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid +WHERE c.relname = 'parent2_id_tenant_key'; + +-- Transaction Group #4 ALTER TABLE child ADD CONSTRAINT child_parent_id_tenant_fkey FOREIGN KEY (parent_id, tenant) REFERENCES parent (id, tenant); + +ALTER TABLE child2 +ADD CONSTRAINT child2_parent2_id_tenant_fkey FOREIGN KEY (parent2_id, tenant) REFERENCES parent2 (id, tenant); \ No newline at end of file