Skip to content

fix: order modified foreign keys after added unique constraints #475

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
meesvandongen wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: order modified foreign keys after added unique constraints #475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions internal/diff/topological.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,9 +512,9 @@ func topologicallySortFunctions(functions []*ir.Function) []*ir.Function {
return sortedFunctions
}

// topologicallySortModifiedTables sorts modified tables based on constraint dependencies
// Tables with added UNIQUE/PK constraints that are referenced by other tables' added FKs
// will come before those tables
// topologicallySortModifiedTables sorts modified tables based on constraint dependencies.
// Tables with added UNIQUE/PK constraints that are referenced by other tables' added OR
// modified FKs will come before those tables.
Comment on lines +515 to +517
func topologicallySortModifiedTables(tableDiffs []*tableDiff) []*tableDiff {
if len(tableDiffs) <= 1 {
return tableDiffs
Expand All @@ -539,12 +539,12 @@ func topologicallySortModifiedTables(tableDiffs []*tableDiff) []*tableDiff {
adjList[key] = []string{}
}

// Build edges: if tableA adds a FK to tableB's newly-added UNIQUE/PK, add edge tableB -> tableA
// Build edges: if tableA adds or modifies a FK to tableB's newly-added UNIQUE/PK,
// add edge tableB -> tableA.
for keyA, tdA := range tableDiffMap {
// Look at FK constraints being added to tableA
for _, fkConstraint := range tdA.AddedConstraints {
if fkConstraint.Type != ir.ConstraintTypeForeignKey {
continue
processFKDependency := func(fkConstraint *ir.Constraint) {
if fkConstraint == nil || fkConstraint.Type != ir.ConstraintTypeForeignKey {
return
}

// Build referenced table key
Expand All @@ -557,7 +557,7 @@ func topologicallySortModifiedTables(tableDiffs []*tableDiff) []*tableDiff {
// Check if referenced table exists in our modified tables set
tdB, exists := tableDiffMap[keyB]
if !exists || keyA == keyB {
continue
return
}

// Check if tableB is adding a UNIQUE or PK constraint that matches the FK reference
Expand All @@ -566,15 +566,21 @@ func topologicallySortModifiedTables(tableDiffs []*tableDiff) []*tableDiff {
continue
}

// Check if this constraint matches the FK's referenced columns
if constraintMatchesFKReference(constraint, fkConstraint) {
// Add edge: tableB (with new UNIQUE/PK) -> tableA (with new FK)
adjList[keyB] = append(adjList[keyB], keyA)
inDegree[keyA]++
break
return
}
Comment on lines 569 to 573

@greptile-apps greptile-apps Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Match referenced column sets

This still only adds the dependency edge when the referenced column order exactly matches the new UNIQUE/PK order. PostgreSQL allows a composite FK such as REFERENCES parent (b, a) to use UNIQUE (a, b), and the existing FK recreation planner already treats this as an unordered column set. When a modified FK references the newly added key in the opposite order, constraintMatchesFKReference returns false, so the dependent table can still be ordered before the referenced table and the migration can fail before the unique constraint exists.

Suggested change
if constraintMatchesFKReference(constraint, fkConstraint) {
// Add edge: tableB (with new UNIQUE/PK) -> tableA (with new FK)
adjList[keyB] = append(adjList[keyB], keyA)
inDegree[keyA]++
break
return
}
if fkReferencesAnyConstraint(fkConstraint, []*ir.Constraint{constraint}) {
adjList[keyB] = append(adjList[keyB], keyA)
inDegree[keyA]++
return
}

}
}

for _, fkConstraint := range tdA.AddedConstraints {
processFKDependency(fkConstraint)
}

for _, constraintDiff := range tdA.ModifiedConstraints {
processFKDependency(constraintDiff.New)
}
Comment on lines +581 to +583
}

// Kahn's algorithm with deterministic cycle breaking
Expand Down
37 changes: 37 additions & 0 deletions internal/diff/topological_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,3 +439,40 @@ func TestBuildFunctionBodyDependenciesWithTopologicalSort(t *testing.T) {
t.Errorf("expected a_helper before z_wrapper, got order: %v", order)
}
}

func TestTopologicallySortModifiedTables_ModifiedForeignKeyDependsOnAddedUnique(t *testing.T) {
referenced := &tableDiff{
Table: &ir.Table{Schema: "public", Name: "organization_role"},
AddedConstraints: []*ir.Constraint{{
Name: "organization_role_organization_id_id_key",
Type: ir.ConstraintTypeUnique,
Columns: []*ir.ConstraintColumn{
{Name: "organization_id", Position: 1},
{Name: "id", Position: 2},
},
}},
}

dependent := &tableDiff{
Table: &ir.Table{Schema: "public", Name: "member_role"},
ModifiedConstraints: []*ConstraintDiff{{
New: &ir.Constraint{
Name: "member_role_organization_id_role_id_fkey",
Type: ir.ConstraintTypeForeignKey,
ReferencedTable: "organization_role",
ReferencedColumns: []*ir.ConstraintColumn{
{Name: "organization_id", Position: 1},
{Name: "id", Position: 2},
},
},
}},
}

sorted := topologicallySortModifiedTables([]*tableDiff{dependent, referenced})
if len(sorted) != 2 {
t.Fatalf("expected 2 modified tables, got %d", len(sorted))
}
if sorted[0].Table.Name != "organization_role" || sorted[1].Table.Name != "member_role" {
t.Fatalf("expected referenced table before dependent modified FK table, got [%s %s]", sorted[0].Table.Name, sorted[1].Table.Name)
}
}
9 changes: 9 additions & 0 deletions testdata/diff/dependency/issue_modified_fk_to_new_unique/diff.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
ALTER TABLE parent_variants
ADD CONSTRAINT parent_variants_parent_entity_id_id_key UNIQUE (parent_entity_id, id);

ALTER TABLE child_links DROP CONSTRAINT child_links_parent_variant_fkey;

ALTER TABLE child_links
ADD CONSTRAINT child_links_parent_variant_fkey FOREIGN KEY (parent_entity_id, parent_variant_id) REFERENCES parent_variants (parent_entity_id, id) ON DELETE CASCADE NOT VALID;

ALTER TABLE child_links VALIDATE CONSTRAINT child_links_parent_variant_fkey;
20 changes: 20 additions & 0 deletions testdata/diff/dependency/issue_modified_fk_to_new_unique/new.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
CREATE TABLE parent_entities (
id integer PRIMARY KEY
);

CREATE TABLE parent_variants (
id integer PRIMARY KEY,
parent_entity_id integer NOT NULL,
CONSTRAINT parent_variants_parent_entity_id_fkey FOREIGN KEY (parent_entity_id)
REFERENCES parent_entities (id)
ON DELETE CASCADE,
CONSTRAINT parent_variants_parent_entity_id_id_key UNIQUE (parent_entity_id, id)
);

CREATE TABLE child_links (
parent_variant_id integer NOT NULL,
parent_entity_id integer NOT NULL,
CONSTRAINT child_links_parent_variant_fkey FOREIGN KEY (parent_entity_id, parent_variant_id)
REFERENCES parent_variants (parent_entity_id, id)
ON DELETE CASCADE
);
19 changes: 19 additions & 0 deletions testdata/diff/dependency/issue_modified_fk_to_new_unique/old.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
CREATE TABLE parent_entities (
id integer PRIMARY KEY
);

CREATE TABLE parent_variants (
id integer PRIMARY KEY,
parent_entity_id integer NOT NULL,
CONSTRAINT parent_variants_parent_entity_id_fkey FOREIGN KEY (parent_entity_id)
REFERENCES parent_entities (id)
ON DELETE CASCADE
);

CREATE TABLE child_links (
parent_variant_id integer NOT NULL,
parent_entity_id integer NOT NULL,
CONSTRAINT child_links_parent_variant_fkey FOREIGN KEY (parent_variant_id)
REFERENCES parent_variants (id)
ON DELETE CASCADE
);
38 changes: 38 additions & 0 deletions testdata/diff/dependency/issue_modified_fk_to_new_unique/plan.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
{
"version": "1.0.0",
"pgschema_version": "1.11.0",
"created_at": "1970-01-01T00:00:00Z",
"source_fingerprint": {
"hash": "cf492aa24c7a6ee879a81c2015c4b84a55981752bb21cc45ad7b02df4efe9f20"
},
"groups": [
{
"steps": [
{
"sql": "ALTER TABLE parent_variants\nADD CONSTRAINT parent_variants_parent_entity_id_id_key UNIQUE (parent_entity_id, id);",
"type": "table.constraint",
"operation": "create",
"path": "public.parent_variants.parent_variants_parent_entity_id_id_key"
},
{
"sql": "ALTER TABLE child_links DROP CONSTRAINT child_links_parent_variant_fkey;",
"type": "table.constraint",
"operation": "drop",
"path": "public.child_links.child_links_parent_variant_fkey"
},
{
"sql": "ALTER TABLE child_links\nADD CONSTRAINT child_links_parent_variant_fkey FOREIGN KEY (parent_entity_id, parent_variant_id) REFERENCES parent_variants (parent_entity_id, id) ON DELETE CASCADE NOT VALID;",
"type": "table.constraint",
"operation": "create",
"path": "public.child_links.child_links_parent_variant_fkey"
},
{
"sql": "ALTER TABLE child_links VALIDATE CONSTRAINT child_links_parent_variant_fkey;",
"type": "table.constraint",
"operation": "create",
"path": "public.child_links.child_links_parent_variant_fkey"
}
]
}
]
}
9 changes: 9 additions & 0 deletions testdata/diff/dependency/issue_modified_fk_to_new_unique/plan.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
ALTER TABLE parent_variants
ADD CONSTRAINT parent_variants_parent_entity_id_id_key UNIQUE (parent_entity_id, id);

ALTER TABLE child_links DROP CONSTRAINT child_links_parent_variant_fkey;

ALTER TABLE child_links
ADD CONSTRAINT child_links_parent_variant_fkey FOREIGN KEY (parent_entity_id, parent_variant_id) REFERENCES parent_variants (parent_entity_id, id) ON DELETE CASCADE NOT VALID;

ALTER TABLE child_links VALIDATE CONSTRAINT child_links_parent_variant_fkey;
24 changes: 24 additions & 0 deletions testdata/diff/dependency/issue_modified_fk_to_new_unique/plan.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
Plan: 2 to modify.

Summary by type:
tables: 2 to modify

Tables:
~ child_links
- child_links_parent_variant_fkey (constraint)
+ child_links_parent_variant_fkey (constraint)
~ parent_variants
+ parent_variants_parent_entity_id_id_key (constraint)

DDL to be executed:
--------------------------------------------------

ALTER TABLE parent_variants
ADD CONSTRAINT parent_variants_parent_entity_id_id_key UNIQUE (parent_entity_id, id);

ALTER TABLE child_links DROP CONSTRAINT child_links_parent_variant_fkey;

ALTER TABLE child_links
ADD CONSTRAINT child_links_parent_variant_fkey FOREIGN KEY (parent_entity_id, parent_variant_id) REFERENCES parent_variants (parent_entity_id, id) ON DELETE CASCADE NOT VALID;

ALTER TABLE child_links VALIDATE CONSTRAINT child_links_parent_variant_fkey;
9 changes: 9 additions & 0 deletions testdata/diff/dependency/issue_modified_fk_to_new_unique_reverse_order/diff.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
ALTER TABLE z_parent_variants
ADD CONSTRAINT z_parent_variants_parent_entity_id_id_key UNIQUE (parent_entity_id, id);

ALTER TABLE a_child_links DROP CONSTRAINT a_child_links_parent_variant_fkey;

ALTER TABLE a_child_links
ADD CONSTRAINT a_child_links_parent_variant_fkey FOREIGN KEY (parent_entity_id, parent_variant_id) REFERENCES z_parent_variants (parent_entity_id, id) ON DELETE CASCADE NOT VALID;

ALTER TABLE a_child_links VALIDATE CONSTRAINT a_child_links_parent_variant_fkey;
20 changes: 20 additions & 0 deletions testdata/diff/dependency/issue_modified_fk_to_new_unique_reverse_order/new.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
CREATE TABLE z_parent_entities (
id integer PRIMARY KEY
);

CREATE TABLE z_parent_variants (
id integer PRIMARY KEY,
parent_entity_id integer NOT NULL,
CONSTRAINT z_parent_variants_parent_entity_id_fkey FOREIGN KEY (parent_entity_id)
REFERENCES z_parent_entities (id)
ON DELETE CASCADE,
CONSTRAINT z_parent_variants_parent_entity_id_id_key UNIQUE (parent_entity_id, id)
);

CREATE TABLE a_child_links (
parent_variant_id integer NOT NULL,
parent_entity_id integer NOT NULL,
CONSTRAINT a_child_links_parent_variant_fkey FOREIGN KEY (parent_entity_id, parent_variant_id)
REFERENCES z_parent_variants (parent_entity_id, id)
ON DELETE CASCADE
);
19 changes: 19 additions & 0 deletions testdata/diff/dependency/issue_modified_fk_to_new_unique_reverse_order/old.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
CREATE TABLE z_parent_entities (
id integer PRIMARY KEY
);

CREATE TABLE z_parent_variants (
id integer PRIMARY KEY,
parent_entity_id integer NOT NULL,
CONSTRAINT z_parent_variants_parent_entity_id_fkey FOREIGN KEY (parent_entity_id)
REFERENCES z_parent_entities (id)
ON DELETE CASCADE
);

CREATE TABLE a_child_links (
parent_variant_id integer NOT NULL,
parent_entity_id integer NOT NULL,
CONSTRAINT a_child_links_parent_variant_fkey FOREIGN KEY (parent_variant_id)
REFERENCES z_parent_variants (id)
ON DELETE CASCADE
);
38 changes: 38 additions & 0 deletions testdata/diff/dependency/issue_modified_fk_to_new_unique_reverse_order/plan.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
{
"version": "1.0.0",
"pgschema_version": "1.11.0",
"created_at": "1970-01-01T00:00:00Z",
"source_fingerprint": {
"hash": "e0b0e7a0420b2e22309e50c6a79beb8a8467358c23e0c8b4fb4f6d30cb1de8f3"
},
"groups": [
{
"steps": [
{
"sql": "ALTER TABLE z_parent_variants\nADD CONSTRAINT z_parent_variants_parent_entity_id_id_key UNIQUE (parent_entity_id, id);",
"type": "table.constraint",
"operation": "create",
"path": "public.z_parent_variants.z_parent_variants_parent_entity_id_id_key"
},
{
"sql": "ALTER TABLE a_child_links DROP CONSTRAINT a_child_links_parent_variant_fkey;",
"type": "table.constraint",
"operation": "drop",
"path": "public.a_child_links.a_child_links_parent_variant_fkey"
},
{
"sql": "ALTER TABLE a_child_links\nADD CONSTRAINT a_child_links_parent_variant_fkey FOREIGN KEY (parent_entity_id, parent_variant_id) REFERENCES z_parent_variants (parent_entity_id, id) ON DELETE CASCADE NOT VALID;",
"type": "table.constraint",
"operation": "create",
"path": "public.a_child_links.a_child_links_parent_variant_fkey"
},
{
"sql": "ALTER TABLE a_child_links VALIDATE CONSTRAINT a_child_links_parent_variant_fkey;",
"type": "table.constraint",
"operation": "create",
"path": "public.a_child_links.a_child_links_parent_variant_fkey"
}
]
}
]
}
9 changes: 9 additions & 0 deletions testdata/diff/dependency/issue_modified_fk_to_new_unique_reverse_order/plan.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
ALTER TABLE z_parent_variants
ADD CONSTRAINT z_parent_variants_parent_entity_id_id_key UNIQUE (parent_entity_id, id);

ALTER TABLE a_child_links DROP CONSTRAINT a_child_links_parent_variant_fkey;

ALTER TABLE a_child_links
ADD CONSTRAINT a_child_links_parent_variant_fkey FOREIGN KEY (parent_entity_id, parent_variant_id) REFERENCES z_parent_variants (parent_entity_id, id) ON DELETE CASCADE NOT VALID;

ALTER TABLE a_child_links VALIDATE CONSTRAINT a_child_links_parent_variant_fkey;
24 changes: 24 additions & 0 deletions testdata/diff/dependency/issue_modified_fk_to_new_unique_reverse_order/plan.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
Plan: 2 to modify.

Summary by type:
tables: 2 to modify

Tables:
~ a_child_links
- a_child_links_parent_variant_fkey (constraint)
+ a_child_links_parent_variant_fkey (constraint)
~ z_parent_variants
+ z_parent_variants_parent_entity_id_id_key (constraint)

DDL to be executed:
--------------------------------------------------

ALTER TABLE z_parent_variants
ADD CONSTRAINT z_parent_variants_parent_entity_id_id_key UNIQUE (parent_entity_id, id);

ALTER TABLE a_child_links DROP CONSTRAINT a_child_links_parent_variant_fkey;

ALTER TABLE a_child_links
ADD CONSTRAINT a_child_links_parent_variant_fkey FOREIGN KEY (parent_entity_id, parent_variant_id) REFERENCES z_parent_variants (parent_entity_id, id) ON DELETE CASCADE NOT VALID;

ALTER TABLE a_child_links VALIDATE CONSTRAINT a_child_links_parent_variant_fkey;
Loading