Skip to content

fix: keep inherited constraint index names when dumping partitioned tables (#653) - #663

Merged
tianzhou merged 4 commits into
mainfrom
fix/issue-653-inherited-constraint-index-names
Oct 27, 2026
Merged

tianzhou merged 4 commits into
mainfrom
fix/issue-653-inherited-constraint-index-names

Conversation

@tianzhou

Copy link
Copy Markdown
Contributor

Fixes #653

Problem

PARTITION OF copies a parent's inline PRIMARY KEY and UNIQUE constraints onto the new partition under generated names. Since #643 a dump creates partitions before the standalone indexes, so when a standalone index holds such a name (for example events_leaf_pkey), the copy takes it:

  • CREATE INDEX IF NOT EXISTS events_leaf_pkey is skipped with a notice and the following ALTER INDEX ... ATTACH PARTITION fails with SQLSTATE 42809.
  • When a child's key index was renamed and its original name reused for an index on another column, replay succeeds but silently drops that index.
  • pgschema apply of its own dump fails the same way, because the desired state is loaded from the same SQL.

All three were reproduced on main (1.13.3) against PostgreSQL 17.6 with the SQL from the issue.

Fix

Follow pg_dump. A PRIMARY KEY or UNIQUE constraint on a partitioned table that has partitions in the output is no longer written inline. After all tables exist, the dump emits one step per constraint:

ALTER TABLE ONLY events ADD CONSTRAINT events_pkey PRIMARY KEY (id);
ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_pkey1 PRIMARY KEY (id);
ALTER INDEX events_pkey ATTACH PARTITION events_leaf_pkey1;

ONLY stops PostgreSQL from creating partition copies under generated names; each copy is added under its catalog name and attached explicitly, nested partitions included. Key columns carry NOT NULL explicitly, as pg_dump writes them. Foreign keys referencing the table or any of its partitions wait for these constraints, and the standalone partitioned indexes from #643 follow them (they may attach to constraint-backed children).

  • The inspector records the partition copies' names (pg_constraint.conparentid, contype in p/u) in Table.InheritedConstraints. It is inspection metadata, excluded from serialization and fingerprints. If a name is missing, the constraint stays inline as before.
  • Migration generation applies the same ordering to complete new hierarchies, which planPartitionIndexCreation already selects for explicit index attachment. Existing hierarchies are unchanged.
  • Multi-file dumps write each constraint family under constraints/, like postponed foreign keys. Docs updated.

Compared with #645, this needs no runtime name lookups, no DO blocks, and no changes to the desired-state loader.

Tests

  • testdata/diff/create_table/issue_653_inherited_constraint_index_name: plan and apply of a new hierarchy with a colliding standalone index, a nested partition, a partitioned UNIQUE, and foreign keys to the parent and to a partition.
  • testdata/dump/issue_653_inherited_constraint_index_name: dump fixture from pg_dump output, including the renamed-key variant.
  • TestDumpCommand_PartitionIndexIdentity gains a partitioned table with PRIMARY KEY and UNIQUE whose generated names are taken, nested partitions, a renamed copy, and a referencing foreign key. It replays every output mode and compares the index and foreign-key catalogs with the source.
  • Refreshed sakila, issue_409_partitioned_fk and issue_472_partition_clone_trigger dump fixtures (key columns gain NOT NULL, constraints move after the tables).
  • Passed locally: go test ./internal/diff ./internal/dump ./internal/plan ./ir/..., go test ./cmd/dump, go test ./cmd on PostgreSQL 18, and the new cases plus Sakila and PartitionIndexIdentity on PostgreSQL 14.

🤖 Generated with Claude Code

…ables (#653)

PARTITION OF copies a parent's inline PRIMARY KEY and UNIQUE constraints onto
the new partition under generated names. A dump created partitions before the
standalone indexes, so when a standalone index held such a name (for example
events_leaf_pkey), the copy took it. The later CREATE INDEX IF NOT EXISTS was
skipped and ALTER INDEX ... ATTACH PARTITION failed with SQLSTATE 42809, or a
same-named index on another column was silently dropped. pgschema apply of its
own dump failed the same way.

Follow pg_dump instead. A PRIMARY KEY or UNIQUE constraint on a partitioned
table with partitions is no longer written inline. After all tables exist the
dump adds it with ALTER TABLE ONLY, adds each partition's copy under its
catalog name the same way, and attaches the copies with ALTER INDEX ... ATTACH
PARTITION, nested partitions included. Key columns carry NOT NULL explicitly.
Foreign keys referencing the table or its partitions wait for these
constraints. The inspector records the partition copies' names as inspection
metadata, excluded from serialization and fingerprints; when a name is missing
the constraint stays inline as before.

Migration generation applies the same ordering to complete new hierarchies,
which planPartitionIndexCreation already selects for explicit index
attachment.

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

greptile-apps Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The changes affect partitioned-table dumps and migrations that create complete new hierarchies.

Fix the SQL-function ordering before merging; valid schemas can otherwise fail during replay or apply.

Findings

  1. P1 Functions precede required primary keys ▶

Summary

Preserves inherited primary-key and unique-constraint names when dumping partitioned tables.

  • New partition hierarchies keep the names of their key indexes.
  • Partitioned table dumps keep key names and add constraints after tables.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Inspect inherited constraint names] --> B[Create tables without postponed keys]
  B --> C[Create SQL functions]
  C --> D[Create named constraint copies]
  D --> E[Attach constraint indexes]
  E --> F[Create standalone partitioned indexes]
  F --> G[Add postponed foreign keys]
  C -. Query needs missing primary key .-> H[Function creation fails]
Loading

Reviews (1) · Last reviewed commit: "fix: keep inherited constraint index nam..." · Reviewed by Greptile

Comment thread internal/diff/diff.go Outdated

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

Constraint-dependent SQL routines can be emitted before deferred keys exist, causing dump replay and new-hierarchy application failures.

1 open issue
1 file not reviewed
  • ir/queries/queries.sql.go: Generated file
What changed in this PR

This PR addresses #653 by preserving inherited primary-key and unique-constraint index names in partitioned schema dumps and complete new-hierarchy migrations, extending the explicit index attachment approach from #643.

Changes:

  • Records inherited constraint names as inspection-only metadata.
  • Creates constraint families explicitly after tables, before standalone partitioned indexes and dependent foreign keys.
  • Adds collision and nested-partition regressions, refreshes dump snapshots, and documents the output format.
File Description
testdata/​dump/​sakila/​pgschema.sql Refreshes deferred payment keys and explicit nullability.
testdata/​dump/​issue_653_inherited_constraint_index_name/​raw.sql Defines collision and renamed-key scenarios.
testdata/​dump/​issue_653_inherited_constraint_index_name/​pgschema.sql Adds expected dump output.
testdata/​dump/​issue_653_inherited_constraint_index_name/​pgdump.sql Supplies the PostgreSQL reference dump.
testdata/​dump/​issue_653_inherited_constraint_index_name/​manifest.json Describes the regression fixture.
testdata/​dump/​issue_472_partition_clone_trigger/​pgschema.sql Refreshes deferred ledger key output.
testdata/​dump/​issue_409_partitioned_fk/​pgschema.sql Refreshes key and foreign-key ordering.
testdata/​diff/​create_table/​issue_653_inherited_constraint_index_name/​plan.txt Adds expected readable migration plan.
testdata/​diff/​create_table/​issue_653_inherited_constraint_index_name/​plan.sql Adds expected executable plan.
testdata/​diff/​create_table/​issue_653_inherited_constraint_index_name/​plan.json Adds expected structured plan.
testdata/​diff/​create_table/​issue_653_inherited_constraint_index_name/​old.sql Sets an empty starting schema.
testdata/​diff/​create_table/​issue_653_inherited_constraint_index_name/​new.sql Defines the target hierarchy and referencing tables.
testdata/​diff/​create_table/​issue_653_inherited_constraint_index_name/​diff.sql Adds expected migration statements.
ir/​queries/​queries.sql.go Adds the inherited-constraint query binding.
ir/​queries/​queries.sql Queries inherited constraint names and parent relationships.
ir/​ir.go Adds transient inherited-constraint metadata.
ir/​inspector.go Populates inherited constraint names.
ir/​index_test.go Checks metadata stays out of serialization.
internal/​diff/​table.go Omits deferred inline keys and preserves NOT NULL.
internal/​diff/​partition_index.go Plans and emits named constraint families and attachments.
internal/​diff/​diff.go Schedules deferred families before partitioned indexes.
docs/​cli/​dump.mdx Documents explicit constraint creation and output files.
cmd/​dump/​partition_index_integration_test.go Extends replay coverage for constraint-index collisions.
cmd/​dump/​dump_integration_test.go Registers the new exact-match dump regression.

Review effort: Balanced

Comment thread internal/diff/diff.go
Review of #663 found that SQL-language functions whose bodies are validated
at creation (BEGIN ATOMIC, or check_function_bodies on) and depend on a
postponed key, such as ON CONFLICT ON CONSTRAINT events_pkey, were created
before the key existed.

Each constraint family is now added inside the table batch as soon as its
last partition is created, so it precedes every function scheduled after
that batch. A hierarchy with postponed keys is created in a single batch,
the latest any of its tables needs. The flush before the standalone
partitioned indexes remains for anything not emitted that way.

The diff fixture, the dump fixture and the partition identity test gain a
BEGIN ATOMIC function with ON CONFLICT ON CONSTRAINT on the partitioned key.

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

Dependency ordering can create partitions before their required functions and policies before the primary keys needed to parse them.

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

Review effort: Balanced

Comment thread internal/diff/diff.go Outdated
Comment thread internal/diff/table.go
Review of #663 found two ordering problems with the postponed keys.

Creating a whole hierarchy in its latest batch broke a valid chain: a
partitioned parent, a SQL function reading it, and a partition whose default
calls that function. Tables now keep their own batches. Instead, a function
referencing a hierarchy that completes in a later batch moves after that
batch, since its body may depend on the key, unless a table default or CHECK
or a domain created earlier needs it; then the parent -> function ->
partition order wins.

Policies on tables of such a hierarchy were created with their table, before
the keys, so a policy expression relying on the key (GROUP BY functional
dependency) failed to parse. They now follow the keys. Multi-file dumps keep
them after the constraint file under policies/ rather than bundling them
back into the table file.

A new diff fixture and the partition identity test cover the chain, the
held-back function and the policy.

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

Remaining routine-dependency and cross-table policy ordering defects can break replay or apply for valid schemas.

4 open issues
2 resolved since last review
1 file not reviewed
  • ir/queries/queries.sql.go: Generated file

Review effort: Balanced

Comment thread internal/diff/diff.go
Comment thread internal/diff/diff.go Outdated
Comment thread internal/diff/diff.go Outdated
Comment thread internal/dump/formatter.go Outdated
Review of #663 found three more cases where a valid schema would fail to
replay or apply, plus one that is circular by construction.

A SQL-standard procedure whose body depends on a postponed key was created
before the batch that completes the hierarchy. Procedures now get the same
hold-back as functions.

The exemption that keeps a function in place when a table default or domain
needs it did not follow transitive callees, so a wrapper needed by a
partition default stayed while the function it calls moved later, and the
wrapper then followed it. The needed set now closes over the functions a SQL
body calls.

A policy on another table that relies on the partitioned key was deferred
correctly but bundled back into its table's file in multi-file dumps, ahead
of the constraint file. Any policy the diff emits apart from its table now
keeps that position under policies/, one file per table. The multi-file
hoisting of relation-free function files also keeps a SQL function below
the files of the functions it calls, which the wrapper case exposed.

The chain fixture and the partition identity test cover the wrapper, the
procedure and the cross-table policy.

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

Unresolved routine and consuming-table dependency ordering can still cause dump replay and new-hierarchy apply failures.

1 open issue
1 previously missed issue

In code that hasn't changed since last review

Medium severity Defer unrelated default consumer until function dependencies

internal/​diff/​diff.go:3128

In the new default-function-chain fixture, adding a SQL-standard get_payload() with SELECT payload FROM events GROUP BY id LIMIT 1 as the default of an unrelated settings table marks the function as needed and keeps it ahead of events_pkey, causing replay/apply to fail. Unlike a key-dependent default on the partition itself, this dependency chain is acyclic: partition → key → get_payload() → settings. Reschedule the unrelated consuming table and its function after the key, and add a regression for this case.

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

Review effort: Balanced

Comment thread internal/diff/diff.go
Comment on lines +3172 to +3175
for root, lookup := range lookups {
if completion[root] > target && bodyReferencesRelation(procedure.Definition, lookup) {
target = completion[root]
}
@tianzhou
tianzhou merged commit 906a0f7 into main Oct 27, 2026
2 checks passed
@tianzhou
tianzhou deleted the fix/issue-653-inherited-constraint-index-names branch October 27, 2026 15:57
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.

Partitioned schema dumps can fail to replay or omit indexes when inherited index names change

2 participants