Skip to content

fix(pg-topo): break inline foreign-key cycles instead of dropping statements#314

Open
avallete wants to merge 2 commits into
mainfrom
claude/issue-311-investigation-xl775t
Open

fix(pg-topo): break inline foreign-key cycles instead of dropping statements#314
avallete wants to merge 2 commits into
mainfrom
claude/issue-311-investigation-xl775t

Conversation

@avallete

@avallete avallete commented Jun 26, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #311. A mutual foreign-key reference between tables is a genuine dependency cycle that Kahn's algorithm cannot order, so analyzeAndSort previously dropped the cycle participants — and every statement downstream of them — from the ordered result, leaving only a CYCLE_DETECTED diagnostic. The declarative apply engine then built an incomplete shadow schema from the truncated list, which pg-delta diffed into spurious DROP statements for objects that still existed.

The fix mirrors pg_dump: when a cycle is detected, cross-cycle FK constraints are deferred into standalone ALTER TABLE ... ADD CONSTRAINT statements so the statement set becomes acyclic and applies cleanly.

Key Changes

  • New module src/rewrite/break-fk-cycles.ts:

    • Identifies CREATE TABLE statements participating in each cycle.
    • Strips FK constraints pointing at other tables in the same cycle (self-referential FKs remain inline — PostgreSQL accepts those within a single CREATE TABLE).
    • Re-emits each stripped FK as a standalone ALTER TABLE ... ADD CONSTRAINT, moving the constraint AST verbatim so the name, columns, and ON DELETE/ON UPDATE actions are preserved.
    • Column-level FKs (which omit fk_attrs because the column is implicit) gain an explicit fk_attrs naming the owning column.
    • Handles both table-level and column-level FK constraints, and cycles spanning any number of tables.
  • Updated src/analyze-and-sort.ts:

    • Factored statement-node building into buildStatementNode / buildStatementNodes for reuse after rewriting.
    • Runs the FK-cycle breaker only when a cycle is present, then re-derives the dependency graph from the rewritten statement set.
    • Safety net: any statement still left in an unbreakable (non-FK) cycle is now appended to ordered in deterministic order rather than being dropped, so consumers always receive every statement plus the CYCLE_DETECTED diagnostic.
  • Tests + changeset:

    • test/cycle-fk-split.test.ts: statement preservation, cycle resolution, cross-table FK deferral vs. inline self-FK, and ordering.
    • test/cycle-fk-split.validation.test.ts: Docker-backed PostgreSQL validation that the rewritten order applies cleanly.
    • Changeset (@supabase/pg-topo patch).

Notes for reviewers

  • Synthetic ALTER statements reuse the owning statement's file path with a collision-free statementIndex, so the declarative-apply per-input file-path remap keeps working.
  • The constraint AST node is moved verbatim, so no FK option is lost.
  • The Supabase CLI currently pins pg-delta@1.0.0-alpha.20; shipping this to users needs a pg-topo release plus a pg-delta/CLI bump (downstream follow-up).

🤖 Generated with Claude Code

https://claude.ai/code/session_01DSsCzpn9AZC52rUj8tmjHz

claude added 2 commits June 26, 2026 08:56
Reproduces #311: a mutual foreign-key cycle between two
tables makes analyzeAndSort drop the cycle participants and every dependent
statement from `ordered`, leaving only a CYCLE_DETECTED diagnostic.

RED against current code:
  ordered.length: 1 (expected 5+2)
  CYCLE_DETECTED: 1 (expected 0)
  cycleGroups: 1 (expected 0)
  ALTER_TABLE count: 0 (expected 2)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DSsCzpn9AZC52rUj8tmjHz
…tements

A mutual FK reference between tables is a genuine dependency cycle that
Kahn's algorithm cannot order, so the previous code dropped the cycle
participants — and every statement downstream of them — from `ordered`,
keeping only a CYCLE_DETECTED diagnostic. The declarative apply engine then
built an incomplete shadow schema from the truncated statement list, which
pg-delta diffed into spurious DROP statements for objects that still existed
(#311).

Fix, mirroring pg_dump:
- New src/rewrite/break-fk-cycles.ts detects FK cycles among CREATE TABLE
  statements and defers each cross-cycle FK into a standalone
  ALTER TABLE ... ADD CONSTRAINT (moving the constraint AST verbatim, so the
  name, columns, and ON DELETE/ON UPDATE actions are preserved). Column-level
  FKs gain an explicit fk_attrs naming the owning column. Self-referential
  FKs stay inline.
- analyzeAndSort runs the breaker when a cycle is present and re-derives the
  graph from the rewritten statement set.
- As a safety net, any statement still left in an unbreakable cycle is now
  appended to `ordered` in deterministic order rather than dropped, so
  consumers always receive every statement.

GREEN: the regression test's issue schema now yields ordered.length 7
(5 originals + 2 deferred ALTERs), cycleGroups 0, CYCLE_DETECTED 0,
ALTER_TABLE 2.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DSsCzpn9AZC52rUj8tmjHz
@changeset-bot

changeset-bot Bot commented Jun 26, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: cb8406a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@supabase/pg-topo Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@avallete avallete changed the title Break inline foreign-key cycles (pg_dump style) fix(pg-topo): break inline foreign-key cycles instead of dropping statements Jun 26, 2026
@pkg-pr-new

pkg-pr-new Bot commented Jun 26, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/supabase/pg-toolbelt/@supabase/pg-delta@314
npm i https://pkg.pr.new/supabase/pg-toolbelt/@supabase/pg-topo@314

commit: cb8406a

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.

Cycle in schema causing unpredictable breaking diff

2 participants