fix: defer new table inline FK when it depends on a new unique constraint (#506)

[tianzhou]

Summary

• When a new table's inline FK references columns on a pre-existing table that is gaining a UNIQUE/PK constraint or unique index in the same migration, the CREATE TABLE was emitted before the constraint it depends on (SQLSTATE 42830)
• Extended planFKRecreationForReplacedConstraints to also detect newly-added UNIQUE/PK constraints and unique indexes on modified tables (not just replaced ones)
• FKs matching this pattern are suppressed from inline CREATE TABLE and deferred to after the modify phase via fkPostAdds
• Added fkReferencesAnyUniqueIndex helper for variant 2 (unique expressed as CREATE UNIQUE INDEX instead of table constraint)

Test plan

• Added create_table/issue_506_fk_depends_on_new_unique test fixture (variant 1: unique as table constraint)
• Diff test passes — FK correctly deferred after UNIQUE constraint
• Plan-and-apply test passes — migration applies successfully
• All existing diff tests pass
• Full cmd test suite running

Closes #506

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR fixes a migration ordering bug where a new table's inline FOREIGN KEY was emitted inside CREATE TABLE before the UNIQUE/PK constraint it depends on was added to an existing table in the same migration, producing SQLSTATE 42830.

• planFKRecreationForReplacedConstraints is extended to also track newly-added (not just replaced) UNIQUE/PK constraints and unique indexes on modified tables; FKs from newly-created tables that reference these are suppressed from inline CREATE TABLE and deferred to a postAdds step after the modify phase.
• fkReferencesAnyUniqueIndex is added as a new helper to match an FK's referenced columns against CREATE UNIQUE INDEX-style unique indexes (as opposed to table constraints).
• A test fixture (create_table/issue_506_fk_depends_on_new_unique) is added covering the constraint variant, with passing diff and plan-and-apply tests.

Confidence Score: 4/5

Safe to merge for the constraint variant — the fix is correct and tested. The unique-index path (fkReferencesAnyUniqueIndex) introduced for variant 2 is untested, so that code path could have latent issues without detection.

The constraint-based fix works correctly and is validated by a passing test fixture. Two gaps remain: the CREATE UNIQUE INDEX path (variant 2) has no test fixture, and fkReferencesAnyUniqueIndex doesn't guard against partial indexes. Neither issue breaks existing behavior, but the untested code path means regressions there would go undetected.

internal/diff/table.go — specifically the new fkReferencesAnyUniqueIndex helper and the addedUniqueIndexes population loop, which lack test coverage.

Important Files Changed

Filename	Overview
internal/diff/table.go	Core logic change: extends planFKRecreationForReplacedConstraints to detect newly-added UNIQUE/PK constraints and unique indexes on modified tables; adds fkReferencesAnyUniqueIndex helper. The unique-index path is untested and doesn't filter partial indexes.
testdata/diff/create_table/issue_506_fk_depends_on_new_unique/diff.sql	Expected diff output correctly defers FK creation after the UNIQUE constraint is added to the parent table.
testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.json	Plan correctly sequences CREATE TABLE child → ADD CONSTRAINT UNIQUE on parent → ADD CONSTRAINT FK on child.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Planner as planFKRecreationForReplacedConstraints
    participant MT as modifiedTables loop
    participant AT as addedTables loop
    participant Emit as Migration Emitter

    Planner->>MT: scan DroppedConstraints/ModifiedConstraints → replaced[]
    Planner->>MT: scan AddedConstraints (UNIQUE/PK) → addedConstraints[]
    Planner->>MT: scan AddedIndexes (UNIQUE) → addedUniqueIndexes[]
    Note over Planner: early-exit if all three maps empty

    Planner->>AT: for each new table's FK
    AT->>AT: fkReferencesAnyConstraint(fk, replaced[refKey])?
    AT->>AT: fkReferencesAnyConstraint(fk, addedConstraints[refKey])?
    AT->>AT: fkReferencesAnyUniqueIndex(fk, addedUniqueIndexes[refKey])?
    AT-->>Planner: suppressedInlineFKs + postAdds

    Planner-->>Emit: preDrops (drop FKs before modify)
    Planner-->>Emit: suppressedInlineFKs (omit from CREATE TABLE)
    Planner-->>Emit: postAdds (ADD CONSTRAINT after modify phase)

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Planner as planFKRecreationForReplacedConstraints
    participant MT as modifiedTables loop
    participant AT as addedTables loop
    participant Emit as Migration Emitter

    Planner->>MT: scan DroppedConstraints/ModifiedConstraints → replaced[]
    Planner->>MT: scan AddedConstraints (UNIQUE/PK) → addedConstraints[]
    Planner->>MT: scan AddedIndexes (UNIQUE) → addedUniqueIndexes[]
    Note over Planner: early-exit if all three maps empty

    Planner->>AT: for each new table's FK
    AT->>AT: fkReferencesAnyConstraint(fk, replaced[refKey])?
    AT->>AT: fkReferencesAnyConstraint(fk, addedConstraints[refKey])?
    AT->>AT: fkReferencesAnyUniqueIndex(fk, addedUniqueIndexes[refKey])?
    AT-->>Planner: suppressedInlineFKs + postAdds

    Planner-->>Emit: preDrops (drop FKs before modify)
    Planner-->>Emit: suppressedInlineFKs (omit from CREATE TABLE)
    Planner-->>Emit: postAdds (ADD CONSTRAINT after modify phase)

Loading

_{Reviews (1): Last reviewed commit: "fix: defer new table inline FK when it d..." | Re-trigger Greptile}

[Copilot]

Pull request overview

This PR fixes an ordering bug in incremental migrations where a new table’s inline foreign key can be emitted before the new UNIQUE/PK constraint or unique index it depends on (triggering SQLSTATE 42830). It extends FK deferral logic so such FKs are suppressed from CREATE TABLE and instead emitted later via deferred constraint creation.

Changes:

• Detect newly-added UNIQUE/PK constraints on modified (pre-existing) tables and defer new-table inline FKs that depend on them.
• Also detect newly-added unique indexes on modified tables and defer dependent new-table inline FKs.
• Add a regression fixture for the table-constraint variant of issue #506.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
internal/diff/table.go	Extends FK deferral planning to consider newly-added unique constraints and unique indexes; adds helper for FK↔unique-index matching.
testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.txt	New expected human-readable plan output for the regression case.
testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.sql	New expected SQL plan output for the regression case.
testdata/diff/create_table/issue_506_fk_depends_on_new_unique/plan.json	New expected JSON plan output for the regression case.
testdata/diff/create_table/issue_506_fk_depends_on_new_unique/old.sql	Old-state fixture: parent table without the needed uniqueness.
testdata/diff/create_table/issue_506_fk_depends_on_new_unique/new.sql	Desired-state fixture: parent gains uniqueness; child table references it.
testdata/diff/create_table/issue_506_fk_depends_on_new_unique/diff.sql	New expected diff output for the regression case.

---

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

[tianzhou]

Re-checked the current head with green CI.

Verdict: ready to merge.

Greptile's remaining note is about follow-up coverage, not a blocker for this fix:

• the constraint-backed ordering bug from issue New table's inline FK ordered before new UNIQUE constraint/index on pre-existing referenced table — apply fails with SQLSTATE 42830 #506 is fixed and covered by the new regression fixture
• the unique-index path was added conservatively on the same dependency-ordering principle
• CI is green on the current head

So from the reviewer side I would merge this as-is, and only add a separate fixture for the unique-index variant later if we want to harden that path further.