Skip to content

fix: recreate dependent FKs when a referenced unique/PK constraint is replaced (#439)#470

Merged
tianzhou merged 5 commits into
mainfrom
fix/issue-439-fk-on-replaced-unique
Jun 10, 2026
Merged

fix: recreate dependent FKs when a referenced unique/PK constraint is replaced (#439)#470
tianzhou merged 5 commits into
mainfrom
fix/issue-439-fk-on-replaced-unique

Conversation

@tianzhou

Copy link
Copy Markdown
Contributor

Summary

Replacing a UNIQUE constraint with a PRIMARY KEY failed at apply with SQLSTATE 2BP01 (cannot drop constraint ... because other objects depend on it) when a foreign key in another table referenced the unique constraint. Postgres binds an FK to the specific unique/PK constraint it was created against, but the plan emitted a bare ALTER TABLE ... DROP CONSTRAINT with no handling of the dependent FK.

The diff now detects desired-state foreign keys whose referenced column set matches a unique/PK constraint being dropped or recreated, and — when the FK itself is unchanged between old and new state — emits a self-contained sequence:

  1. drop the dependent FK(s)
  2. apply the table modifications (drop unique, add PK)
  3. recreate the FK(s) from the desired-state definition

The final state still matches the schema files exactly, so no referential-integrity constraint is silently lost (which is why a DROP CONSTRAINT ... CASCADE mode was rejected on the issue). The apply-rewrite layer automatically upgrades the FK recreation to the online ADD CONSTRAINT ... NOT VALID + VALIDATE CONSTRAINT pattern.

Changed or dropped FKs remain handled by their own table diff; only unchanged FKs are routed through the pre-drop/recreate path.

Fixes #439

Test plan

New fixture testdata/diff/dependency/issue_439_unique_to_pk_fk_dependent/ reproduces the issue scenario with two dependent FKs (one with ON DELETE CASCADE to verify clause preservation on recreation). Before the fix, apply fails with the exact error from the issue; after, apply succeeds and the second plan is empty (idempotent).

PGSCHEMA_TEST_FILTER="dependency/issue_439_unique_to_pk_fk_dependent" go test -v ./internal/diff -run TestDiffFromFiles
PGSCHEMA_TEST_FILTER="dependency/issue_439_unique_to_pk_fk_dependent" go test -v ./cmd -run TestPlanAndApply

Regression: full TestDiffFromFiles suite plus dependency/, create_table/, and online/ TestPlanAndApply categories all pass.

🤖 Generated with Claude Code

… replaced (#439)

Replacing a UNIQUE constraint with a PRIMARY KEY (or otherwise dropping or
recreating a unique/PK constraint) failed with SQLSTATE 2BP01 when a foreign
key in another table was bound to that constraint: the plan emitted a bare
ALTER TABLE ... DROP CONSTRAINT with no handling of the dependent FK.

Detect desired-state foreign keys whose referenced column set matches a
unique/PK constraint being dropped or recreated, and when the FK itself is
unchanged, drop it before the table modifications and recreate it afterwards.
The final state still matches the schema files exactly; changed or dropped
FKs remain handled by their own table diff.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 10, 2026 10:33
@greptile-apps

greptile-apps Bot commented Jun 10, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds handling for FKs that depend on replaced unique or primary-key constraints. The main changes are:

  • Tracks unchanged FKs that reference unique/PK constraints being dropped or recreated.
  • Emits FK drops before modified table DDL and FK recreates after it.
  • Adds an issue 439 fixture covering single-column unchanged dependent FKs.

Confidence Score: 2/5

These issues should be fixed before merging.

  • Replacing a referenced unique/PK can still fail when the FK is changed in the same migration.

  • Adding a new referencing table in the same migration can bind its FK to the old constraint before the replacement runs.

  • Composite FK detection can target the wrong constraint when the same columns appear in a different order.

  • internal/diff/table.go

  • internal/diff/diff.go

Important Files Changed

Filename Overview
internal/diff/table.go Adds dependent-FK detection and drop/recreate SQL generation for replaced unique/PK constraints.
internal/diff/diff.go Wires the dependent-FK handling into migration generation around modified table DDL.

Reviews (1): Last reviewed commit: "fix: recreate dependent FKs when a refer..." | Re-trigger Greptile

Comment thread internal/diff/table.go Outdated
Comment thread internal/diff/table.go
Comment thread internal/diff/table.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes Postgres apply failures when replacing a referenced UNIQUE constraint with a PRIMARY KEY by explicitly dropping and recreating unchanged dependent foreign keys that are bound to the replaced unique/PK constraint (issue #439).

Changes:

  • Detect unchanged foreign keys whose referenced column list matches a unique/PK constraint being dropped/recreated, and route them through a drop → table modification → recreate sequence.
  • Emit pre-modify FK drops and post-modify FK recreations in the migration SQL generation.
  • Add a regression fixture covering the unique→PK replacement with multiple dependent FKs (including ON DELETE CASCADE) and expected plans.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/diff/table.go Adds detection of dependent unchanged FKs and emits drop/recreate SQL around table modifications.
internal/diff/diff.go Wires the new FK drop/recreate sequence into the modify phase of migration generation.
testdata/diff/dependency/issue_439_unique_to_pk_fk_dependent/plan.txt New expected human-readable plan output for the regression scenario.
testdata/diff/dependency/issue_439_unique_to_pk_fk_dependent/plan.sql New expected SQL plan output showing FK drop/recreate and online rewrite steps.
testdata/diff/dependency/issue_439_unique_to_pk_fk_dependent/plan.json New expected JSON plan output for the same regression scenario.
testdata/diff/dependency/issue_439_unique_to_pk_fk_dependent/old.sql Old-state schema reproducing the dependency problem (FKs reference a UNIQUE constraint).
testdata/diff/dependency/issue_439_unique_to_pk_fk_dependent/new.sql New-state schema replacing the UNIQUE with a PRIMARY KEY while keeping FKs unchanged.
testdata/diff/dependency/issue_439_unique_to_pk_fk_dependent/diff.sql New expected diff output showing FK drop/recreate around the constraint replacement.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/diff/table.go
… constraints (#439)

Extends the dependent-FK handling to two scenarios flagged in review:

- An FK that itself changes in the same migration as the unique->PK swap was
  left on the normal table-diff path, where its recreate could run before the
  swap and bind to the old constraint. FKs whose old or new definition targets
  a replaced constraint are now routed through the pre-drop/post-add path and
  removed from their own table diff.

- An FK on a newly added table referencing the replaced constraint was emitted
  inline in CREATE TABLE during the create phase, binding to the old constraint
  before the modify-phase swap. Such FKs are now kept out of CREATE TABLE and
  created after the replacement constraint exists.

Verified against live PostgreSQL that FK-to-unique matching is by column set,
not ordered list (FOREIGN KEY (x, y) REFERENCES t (b, a) is valid against
UNIQUE (a, b)), so the order-insensitive matching is kept.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fold the changed-FK and new-table-FK scenarios into the existing
issue_439_unique_to_pk_fk_dependent fixture: one UNIQUE->PK swap with four
referencing tables covering an unchanged FK, an unchanged FK with ON DELETE
CASCADE, an FK changed in the same migration, and a newly added table whose
FK targets the replaced constraint.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Apply cleanup-review findings to the issue 439 change:

- Reuse generateDeferredConstraintsSQL for the post-add emission instead of
  the near-identical generateAddRecreatedFKsSQL; fkPostAdds is now a
  []*deferredConstraint, removing a duplicated copy of the FK ADD format.

- Replace the post-sort mutation of tableDiff DroppedConstraints and
  ModifiedConstraints with a preDroppedFKSet skip-set consulted at emission
  time in generateAlterTableStatements, matching the existing skip-set
  pattern (constraintDroppedWithColumns, droppedTableSet, preDroppedViews)
  and deleting the bespoke list-rewrite helpers.

- Share constraint identity keys via constraintPathKey.

Generated plans are unchanged; all fixtures pass without regeneration.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread internal/diff/table.go Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@tianzhou tianzhou merged commit dc7f62b into main Jun 10, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dropping constraint cascade

2 participants