fix: partition child PK/UNIQUE constraints cause perpetual plan drift (#495)

[tianzhou]

Summary

Fixes #495 — pgschema plan never converges on partitioned tables because partition child PK/UNIQUE constraints are always detected as modified (perpetual DROP + re-ADD of identical constraints).

Root cause: constraintsEqual() compared NoInherit (connoinherit) for all constraint types, but PostgreSQL sets connoinherit inconsistently for PK/UNIQUE on partition children:

• Standalone constraint (what pgschema apply creates): connoinherit = true
• Auto-created via PARTITION OF (desired state): connoinherit = false

This made the comparator always see a difference, even though the constraint definitions are byte-identical. connoinherit only has semantic meaning for CHECK constraints (the NO INHERIT modifier) — it's meaningless for PK/UNIQUE/FK/EXCLUDE.

Fix: Only compare NoInherit when both constraints are CHECK type.

Test plan

• New convergence test: UNIQUE constraint on partition children (standalone+ATTACH vs PARTITION OF)
• New convergence test: PRIMARY KEY constraint on partition children
• Existing CHECK NO INHERIT test (CHECK (FALSE) NO INHERIT constraints silently dropped during plan/apply #386) still passes
• All 185 diff tests pass
• Schema integration tests pass
• go build ./... and go vet ./... clean

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Fixes perpetual pgschema plan drift on partition children when PK/UNIQUE constraints are semantically identical but differ in PostgreSQL’s pg_constraint.connoinherit bookkeeping. The change narrows constraint equality comparison so NoInherit is only considered for CHECK constraints, and adds diff fixtures to ensure convergence for partition-child UNIQUE and PRIMARY KEY constraints.

Changes:

• Update constraintsEqual() to ignore NoInherit differences except for CHECK constraints (where NO INHERIT is semantically meaningful).
• Add convergence fixtures for partition-child UNIQUE constraints (standalone+ATTACH vs PARTITION OF).
• Add convergence fixtures for partition-child PRIMARY KEY constraints (standalone+ATTACH vs PARTITION OF).

Reviewed changes

Copilot reviewed 9 out of 13 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
internal/diff/constraint.go	Prevents false constraint diffs by only comparing NoInherit for CHECK constraints.
testdata/diff/create_table/issue_495_partition_constraint_convergence/old.sql	Fixture: “applied” state with standalone child UNIQUE constraints + ATTACH.
testdata/diff/create_table/issue_495_partition_constraint_convergence/new.sql	Fixture: desired state using PARTITION OF for children.
testdata/diff/create_table/issue_495_partition_constraint_convergence/plan.txt	Fixture expectation: plan converges with “No changes detected.”
testdata/diff/create_table/issue_495_partition_constraint_convergence/plan.json	Fixture expectation: empty plan JSON (no groups).
testdata/diff/create_table/issue_495_partition_constraint_convergence/plan.sql	Fixture placeholder for generated SQL output (no-op case).
testdata/diff/create_table/issue_495_partition_constraint_convergence/diff.sql	Fixture placeholder for human SQL diff output (no-op case).
testdata/diff/create_table/issue_495_partition_pk_convergence/old.sql	Fixture: “applied” state with standalone child PK constraints + ATTACH.
testdata/diff/create_table/issue_495_partition_pk_convergence/new.sql	Fixture: desired state using PARTITION OF for children.
testdata/diff/create_table/issue_495_partition_pk_convergence/plan.txt	Fixture expectation: plan converges with “No changes detected.”
testdata/diff/create_table/issue_495_partition_pk_convergence/plan.json	Fixture expectation: empty plan JSON (no groups).
testdata/diff/create_table/issue_495_partition_pk_convergence/plan.sql	Fixture placeholder for generated SQL output (no-op case).
testdata/diff/create_table/issue_495_partition_pk_convergence/diff.sql	Fixture placeholder for human SQL diff output (no-op case).

---

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

[greptile-apps]

Greptile Summary

This PR fixes a perpetual plan drift bug where pgschema plan would always emit a DROP + re-ADD for PK/UNIQUE constraints on partition children, even when the schema was already converged. The root cause was that constraintsEqual() compared NoInherit (connoinherit) across all constraint types, but PostgreSQL sets connoinherit = true for standalone constraints (created by pgschema apply + ATTACH PARTITION) and connoinherit = false for constraints auto-created via PARTITION OF, causing an always-false equality check.

• Fix: constraintsEqual() now only evaluates NoInherit when the constraint type is CHECK, which is the only type where the NO INHERIT modifier is semantically meaningful.
• Tests: Two new convergence test cases cover UNIQUE and PRIMARY KEY constraints on partition children, verifying that the ATTACH-based state and the PARTITION OF state are treated as equivalent (plan.txt = \"No changes detected.\").
• Regression safety: The existing CHECK NO INHERIT test (issue CHECK (FALSE) NO INHERIT constraints silently dropped during plan/apply #386) is preserved; NoInherit is still compared for CHECK constraints.

Confidence Score: 5/5

Safe to merge. The change is a minimal one-line guard scoped to a well-understood PostgreSQL behavior difference, and the two new test cases directly reproduce the convergence failure.

The fix is surgically correct: constraintsEqual() already checks type equality earlier (making the old.Type guard sufficient), the NoInherit field has no semantic meaning for PK/UNIQUE/FK/EXCLUDE constraints, and the test data accurately reflects the real PostgreSQL behavior difference between ATTACH PARTITION and PARTITION OF. No regressions are introduced for CHECK NO INHERIT (issue #386).

No files require special attention. internal/diff/constraint.go is the only changed logic file and the change is straightforward.

Important Files Changed

Filename	Overview
internal/diff/constraint.go	Single-line guard change: NoInherit is now only compared for CHECK constraints. The existing type-equality check earlier in constraintsEqual() makes the old.Type guard sufficient.
testdata/diff/create_table/issue_495_partition_constraint_convergence/old.sql	Accurately models the post-apply database state: children created as standalone tables with explicitly named UNIQUE constraints, then attached to the parent.
testdata/diff/create_table/issue_495_partition_constraint_convergence/new.sql	Represents the desired state using PARTITION OF, which produces constraints with connoinherit=false. Both test cases are well-structured convergence checks.
testdata/diff/create_table/issue_495_partition_pk_convergence/old.sql	Same pattern as the UNIQUE convergence test but for PRIMARY KEY constraints. Correctly models the ATTACH-based state.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant User as User (desired state)
    participant Plan as pgschema plan
    participant DB as Target DB (after apply)
    participant Eq as constraintsEqual()

    User->>Plan: "new.sql (PARTITION OF → connoinherit=false)"
    DB->>Plan: "pg_constraint (ATTACH → connoinherit=true)"
    Plan->>Eq: compare p_12.p_band_id_geom_id_key (old vs new)

    Note over Eq: Before fix
    Eq-->>Plan: NoInherit true≠false → NOT equal
    Plan-->>User: DROP + ADD constraint (perpetual drift)

    Note over Eq: After fix
    Eq->>Eq: "old.Type == UNIQUE? Skip NoInherit check"
    Eq-->>Plan: equal (all other fields match)
    Plan-->>User: No changes detected

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 User as User (desired state)
    participant Plan as pgschema plan
    participant DB as Target DB (after apply)
    participant Eq as constraintsEqual()

    User->>Plan: "new.sql (PARTITION OF → connoinherit=false)"
    DB->>Plan: "pg_constraint (ATTACH → connoinherit=true)"
    Plan->>Eq: compare p_12.p_band_id_geom_id_key (old vs new)

    Note over Eq: Before fix
    Eq-->>Plan: NoInherit true≠false → NOT equal
    Plan-->>User: DROP + ADD constraint (perpetual drift)

    Note over Eq: After fix
    Eq->>Eq: "old.Type == UNIQUE? Skip NoInherit check"
    Eq-->>Plan: equal (all other fields match)
    Plan-->>User: No changes detected

Loading

_{Reviews (1): Last reviewed commit: "fix: skip NoInherit comparison for non-C..." | Re-trigger Greptile}