Skip to content

fix: detach and attach partitions when their parent or bound changes (#639) - #668

Merged
tianzhou merged 4 commits into
mainfrom
fix/issue-639-partition-attach-detach
Oct 28, 2026
Merged

tianzhou merged 4 commits into
mainfrom
fix/issue-639-partition-attach-detach

Conversation

@tianzhou

@tianzhou tianzhou commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

pgschema plan reported No changes detected. when a partition was re-attached with a different bound or detached from its parent: diffTables compared columns, constraints, indexes, triggers and policies but never the partition parent or bound, so with nothing else changed the diff was dropped.

The comparator now records a changed parent or bound, and the migration applies it as DETACH + ATTACH with the two halves deliberately far apart (see internal/diff/partition.go):

  • DETACH runs in the drop phase, before tables are dropped and before any partition is created. A child whose old parent goes away leaves it before DROP ... CASCADE, and sibling partitions whose bounds shift against each other, or new partitions created in the space a re-bounded sibling gives up, never overlap mid-apply.
  • ATTACH runs at the end of the table modifications, once the child has the columns and constraints the parent requires.

Two details keep the attach from doing needless or failing work:

  • A detached table keeps the PK/UNIQUE/FK copies it inherited as local constraints, which the desired-state partition does not list. Before the diff, the old state of a table that leaves its parent gains those copies under their catalog names (the inherited-constraint query now records FK copies too), so a standalone table that declares the same constraint keeps it instead of adding it a second time, and one that does not declare it drops it. The query only pairs a constraint with a copy on a partition of its table, so the internal per-partition sub-constraints of a foreign key that references a partitioned table are not mistaken for inherited copies. When the table is re-attached, copies that match the new parent's constraint (columns, deferrability, referential actions) are kept; ATTACH reuses the existing index and foreign key instead of rebuilding and re-validating them.
  • A foreign key that references a table through a key it only gains by being attached, whether on a new table, newly added, or a changed foreign key retargeted to it, is deferred past the ATTACH; the partitions of an attached partitioned table gain the key at the same time and are covered too.
  • Identity inherited from a partition parent (PG17+) is stripped from the child during normalization: DETACH removes it and ATTACH rejects a child that has one of its own, so the plan must not add it to a detached table before re-attaching.

Plan output shows the steps as ~ child (detach partition) / ~ child (attach partition) under the table.

This supersedes the approach in #660, which emitted both statements inline in the ALTER TABLE phase and so hit the ordering problems above.

Fixes #639

Test plan

One case, testdata/diff/create_table/issue_639_partition_attach_detach, with three independent hierarchies drifted on the target:

  • events (identity on the parent, PG17+, foreign key to a partitioned accounts so the FK copies carry internal sub-constraints): one partition re-bounded (the reporter's scenario) and a detached one re-attached without an ADD GENERATED ... AS IDENTITY first, both keeping the PK/FK/CHECK copies they retained; a key-less standalone table attached and referenced both by a new table's foreign key and by an existing foreign key retargeted to it, both deferred past the attach; a partition becoming standalone, keeping the primary key it still declares and dropping the FK and CHECK copies it no longer declares.
  • metrics (two-level hierarchy): the boundary between two leaves moves and a third leaf takes the freed range, every detach preceding the create and the re-attaches; a leaf becomes standalone, its retained primary key resolved through the intermediate partition whose own copy is absent from its IR; a standalone partitioned table is attached and a new table's foreign key to its leaf waits for the attach that gives the leaf its key.
  • old_parent / new_parent: the old parent is dropped while its child stays standalone, and a standalone table is attached to a parent created in the same migration.

Unit tests: TestStripInheritedIdentityNested (ir) for a root → partition → sub-partition hierarchy, TestParentSuppliesConstraint, TestWithConstraintsRetainedByDetach and TestResolveInheritedConstraintNested (internal/diff).

Known limitations, documented in partition.go: DETACH fails when rows of the partition are referenced through a foreign key that points at the parent (dropping and re-validating such foreign keys around the re-attach is left to the operator), and a step that must run on its own between the two halves (CREATE INDEX CONCURRENTLY, VALIDATE CONSTRAINT, a directive) commits the DETACH before the ATTACH, as it does for the foreign keys dropped and recreated around a replaced key.

PGSCHEMA_TEST_FILTER="create_table/issue_639" go test -v ./internal/diff -run TestDiffFromFiles
PGSCHEMA_TEST_FILTER="create_table/issue_639" go test -v ./cmd -run TestPlanAndApply

Run locally: the case above (diff and plan-and-apply, which verifies convergence after apply), the full TestDiffFromFiles suite, TestPlanAndApplyPartitionIndexIdentity, and the internal/plan, ir and internal/fingerprint unit tests, all on PostgreSQL 18. The dump suite was not run locally.

🤖 Generated with Claude Code

…639)

The table comparator never looked at a partition's parent or bound, so a
partition re-attached with a different bound, or detached altogether,
produced an empty plan.

diffTables now records a changed parent or bound, and the migration emits
DETACH PARTITION in the drop phase (before the old parent can be dropped
and before any partition is created, so shifted sibling bounds never
overlap) and ATTACH PARTITION at the end of the table modifications (once
the child has the columns and constraints the parent requires).

Two details keep the attach from doing needless or failing work:
- PK/UNIQUE/FK copies that a detached table kept from its parent are no
  longer dropped when the parent supplies an equivalent; ATTACH reuses them.
- Identity inherited from a partition parent (PG17+) is stripped from the
  child in normalization, since DETACH removes it and ATTACH rejects a
  child that carries its own.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 27, 2026 17:01
@greptile-apps

greptile-apps Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 2/5

[Medium impact] The changes affect PostgreSQL partition migrations.

Fix the retained primary key and both inherited-identity cases before merging.

Findings

  1. P1 Nested identity cleanup changes randomly ▶
  2. P1 Detach adds an existing key ▶
  3. P1 Cross-schema identity attachment fails ▶

Summary

The PR detects partition parent and bound changes and splits migration steps around table changes:

  • Partition changes now create ordered detach and attach steps.
  • Partition children no longer claim their parent's identity.

Three issues remain: detaching with a retained primary key adds it twice, nested identity cleanup depends on map order, and cross-schema identity attachment still adds an identity before attaching.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Compare parent and bound] --> B[Detach changed partitions]
  B --> C[Drop and create tables]
  C --> D[Change columns and constraints]
  D --> E[Attach partitions]
Loading

Reviews (1) · Last reviewed commit: "fix: detach and attach partitions when t..." · Reviewed by Greptile

Comment thread ir/normalize.go Resolved
Comment thread internal/diff/diff.go Resolved
Comment thread ir/normalize.go Resolved

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.

🟡 Changes recommended

Unresolved constraint-transition handling and order-dependent identity normalization can cause failed migrations or incorrect resulting schemas.

5 open issues
What changed in this PR

Addresses #639 by detecting partition parent and bound changes in pgschema’s migration planner. Unlike #660’s inline approach, this separates early detachment from reattachment after table modifications.

Changes:

  • Detects partition transitions and emits ordered DETACH/ATTACH operations.
  • Preserves reusable constraints, normalizes inherited identity metadata, and labels partition changes in plan summaries.
  • Adds four regression fixtures covering bound shifts, detached partitions, parent transitions, and identity inheritance.
File Description
testdata/​diff/​create_table/​issue_639_partition_sibling_shift/​plan.txt Expected sibling-shift summary.
testdata/​diff/​create_table/​issue_639_partition_sibling_shift/​plan.sql Expected ordered migration SQL.
testdata/​diff/​create_table/​issue_639_partition_sibling_shift/​plan.json Structured sibling-shift plan.
testdata/​diff/​create_table/​issue_639_partition_sibling_shift/​old.sql Initial sibling bounds.
testdata/​diff/​create_table/​issue_639_partition_sibling_shift/​new.sql Shifted bounds and new partition.
testdata/​diff/​create_table/​issue_639_partition_sibling_shift/​diff.sql Expected detach/create/attach sequence.
testdata/​diff/​create_table/​issue_639_partition_reparent/​plan.txt Expected parent-transition summary.
testdata/​diff/​create_table/​issue_639_partition_reparent/​plan.sql Expected parent-transition SQL.
testdata/​diff/​create_table/​issue_639_partition_reparent/​plan.json Structured parent-transition plan.
testdata/​diff/​create_table/​issue_639_partition_reparent/​old.sql Original parent and standalone table.
testdata/​diff/​create_table/​issue_639_partition_reparent/​new.sql Desired standalone child and new parent.
testdata/​diff/​create_table/​issue_639_partition_reparent/​diff.sql Expected detach/drop/create/attach sequence.
testdata/​diff/​create_table/​issue_639_partition_rebound_detached/​plan.txt Expected drift-correction summary.
testdata/​diff/​create_table/​issue_639_partition_rebound_detached/​plan.sql Expected reattachment SQL.
testdata/​diff/​create_table/​issue_639_partition_rebound_detached/​plan.json Structured drift-correction plan.
testdata/​diff/​create_table/​issue_639_partition_rebound_detached/​old.sql Rebounded and detached partition drift.
testdata/​diff/​create_table/​issue_639_partition_rebound_detached/​new.sql Desired partition bounds and constraints.
testdata/​diff/​create_table/​issue_639_partition_rebound_detached/​diff.sql Expected partition restoration sequence.
testdata/​diff/​create_table/​issue_639_partition_attach_identity/​plan.txt Expected identity-case summary.
testdata/​diff/​create_table/​issue_639_partition_attach_identity/​plan.sql Attachment without adding child identity.
testdata/​diff/​create_table/​issue_639_partition_attach_identity/​plan.json Structured identity-case plan.
testdata/​diff/​create_table/​issue_639_partition_attach_identity/​old.sql Detached child of an identity-bearing parent.
testdata/​diff/​create_table/​issue_639_partition_attach_identity/​new.sql Desired identity-inheriting partition.
testdata/​diff/​create_table/​issue_639_partition_attach_identity/​diff.sql Expected attachment-only migration.
ir/​normalize.go Strips inherited partition identity markers.
internal/​plan/​plan.go Displays partition detach/attach labels.
internal/​diff/​table.go Includes partition changes in table comparison.
internal/​diff/​partition.go Implements partition comparison, constraint reuse, and SQL generation.
internal/​diff/​diff.go Adds partition operations and migration-phase integration.

Review effort: Balanced

Comment thread internal/diff/diff.go Resolved
Comment thread internal/diff/partition.go Resolved
Comment thread internal/diff/partition.go Resolved
Comment thread internal/diff/partition.go Resolved
Comment thread ir/normalize.go Resolved
…639)

Review of #668 found that a partition which becomes a standalone table
keeps the copies of its parent's PRIMARY KEY, UNIQUE and FOREIGN KEY
constraints as local constraints after DETACH, while the inspector
leaves those copies out of a partition's IR. A desired standalone table
declaring the same primary key was therefore scheduled for a second
ADD CONSTRAINT, which PostgreSQL refuses.

The inherited-constraint query now also records foreign key copies, and
before diffing a table that leaves its parent, the old state gains those
copies under the names the catalog gave them. A same-named declaration
is kept, a missing one is dropped, and a re-attach still reuses the
copies the new parent supplies.

Also from the review:
- stripInheritedIdentity read parent columns it had already cleared, so
  a root → partition → sub-partition hierarchy came out differently
  depending on map iteration order. Identity flags are now snapshotted
  before any are cleared.
- ATTACH adopts a child copy by its columns alone, keeping the copy's
  own deferrability and ON DELETE SET NULL column list, so a copy that
  differs from the parent's constraint in those is no longer kept.

Co-Authored-By: Claude Fable 5.1 <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.

🟡 Changes recommended

Critical partition attach/detach bugs can generate invalid constraint operations or commit detachment before reattachment, causing migration failures and excluding partition rows from parent queries.

3 open issues
2 previously missed issues

In code that hasn't changed since last review

Medium severity Match parent and child constraints one-to-one

internal/​diff/​partition.go:134

This retains every child constraint matching any parent constraint, but ATTACH adopts only one child copy per parent constraint. Attaching a standalone table with two equivalent UNIQUE constraints or foreign keys to a parent with one matching constraint therefore leaves an unwanted local copy, and the next plan still reports a drop. Match parent and child constraints one-to-one, leaving surplus copies scheduled for removal.

Medium severity Preserve independently owned child identities on PostgreSQL 14–16

ir/​normalize.go:1857

On PostgreSQL 14–16, partitions can have independently owned identities rather than inheriting the parent's identity. When parent and child both have an identity on the same column, this normalization erases the child's real definition, so changing or removing it can produce an empty plan. Gate this normalization on PostgreSQL 17+, or distinguish inherited identities using catalog ownership metadata.

5 resolved since last review
1 file not reviewed
  • ir/queries/queries.sql.go: Generated file

Review effort: Balanced

Comment thread internal/diff/diff.go Resolved
Comment thread internal/diff/diff.go Resolved
Comment thread internal/diff/partition.go Resolved
…s past ATTACH (#639)

Second round of review on #668:

- An intermediate partition's IR holds neither its own copy of the
  root's key nor the definition a leaf's copy derives from, so the
  retained-constraint lookup missed it and a standalone leaf declaring
  that primary key was scheduled to add it a second time.
  resolveInheritedConstraint follows InheritedConstraints up the chain;
  both the detach and the attach side use it.
- A foreign key that references a table being attached through a key it
  only gains from its parent now waits for the ATTACH, the same way the
  FK planner already defers FKs that target keys this migration adds.
  The ATTACH moves ahead of the deferred FK batch.
- The transaction split an isolated step causes between DETACH and
  ATTACH is documented alongside the incoming-FK limitation.

Co-Authored-By: Claude Fable 5.1 <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.

🔵 Needs a closer look

Foreign-key planning misses attachment-created key dependencies and misidentifies inherited constraints, causing migrations to fail or attempt invalid constraint changes.

No open issues 🎉

3 previously missed issues

In code that hasn't changed since last review

Medium severity Propagate attached key dependencies to descendant partitions

internal/​diff/​table.go:755

Attaching a standalone partitioned table beneath a parent with a primary key also supplies inherited keys to its existing descendants, but keysFromAttach records only the attached table. A new foreign key referencing a previously unkeyed leaf is therefore emitted before ATTACH creates that leaf's key, and apply fails. Propagate these attachment dependencies through the descendant hierarchy and cover a nested attach with a foreign key referencing a leaf.

Medium severity Include attached keys when deciding to recreate modified foreign keys

internal/​diff/​table.go:834

A same-named foreign key retargeted to a child that gains its primary key through ATTACH is recreated during table modifications, before the referenced key exists, so apply fails. The existing-FK newBound predicate omits keysFromAttach; add fkReferencesAnyConstraint(newFK, keysFromAttach[fkReferencedTableKey(newFK)]) to that predicate so modified foreign keys use the pre-drop/post-add path too, and cover this retargeting case.

Medium severity Exclude internal partitioned-table FK subconstraints from inherited copies

ir/​queries/​queries.sql:1497

Foreign keys with conparentid also include internal subconstraints for references to partitioned tables, where c.conrelid = pc.conrelid, not just copies inherited from a partition parent. These rows can overwrite a genuine inherited copy's name in buildInheritedConstraints, causing a detach plan to drop an internal constraint or add a constraint that already exists. Restrict the query to actual child/parent table relationships through pg_inherits, regenerate queries.sql.go, and cover detaching a partition whose foreign key references a partitioned table.

3 resolved since last review
1 file not reviewed
  • ir/queries/queries.sql.go: Generated file

Review effort: Balanced

…o keys gained by nested or retargeted attach (#639)

Consolidate the seven issue_639 fixtures into one case with three drifted
hierarchies, and address three review findings:

- GetInheritedPartitionConstraintsForSchema joins pg_inherits so the
  internal per-partition sub-constraints of a foreign key that references
  a partitioned table (same table, conparentid set) are not recorded as
  inherited copies.
- keysFromAttach covers the partitions of an attached partitioned table,
  which gain the parent's key from the same ATTACH.
- A changed foreign key retargeted to a key its referenced table only
  gains by being attached is dropped up front and re-added after the
  attach instead of recreated in place before it.

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

Copy link
Copy Markdown
Contributor Author

Re the three "previously missed" items in the latest Copilot summary (no inline threads, so answering here). All three were real and are fixed in 3386a0a, which also consolidates the seven issue_639_* fixtures into one, create_table/issue_639_partition_attach_detach.

  • Descendants of an attached partitioned table (table.go, keysFromAttach): the partitions of a table being attached gain the parent's key from the same ATTACH, so keysFromAttach now propagates down the PartitionOf chain. Covered by the standalone partitioned metrics_high attached under metrics, with a new table whose FK references its leaf metrics_high_a; the FK is deferred past the attach.
  • Changed FK retargeted to an attached table (table.go, newBound): the predicate now consults keysFromAttach, so such an FK takes the pre-drop/post-add path. Covered by audit_log, whose FK moves from events_2026_01 to events_2026_03: dropped up front, re-added after events_2026_03 is attached.
  • FK sub-constraints in the inherited-copy query (queries.sql): verified on PG 18 that an FK referencing a partitioned table owns internal sub-constraints (<fk>_1, ...) on the same table with conparentid set and no pg_inherits link. The query now joins pg_inherits on (inhrelid = c.conrelid, inhparent = pc.conrelid), and queries.sql.go is regenerated. Covered by making accounts partitioned in the fixture, so every FK to it, including the copies on the events partitions that are detached and attached, carries those sub-constraints.

@tianzhou
tianzhou merged commit 36a38ff into main Oct 28, 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.

plan ignores partition bounds and attachment: a re-bounded or detached partition gives No changes detected

2 participants