Skip to content

fix: preserve constraint INCLUDE columns (#673) - #676

Open
zikolach wants to merge 1 commit into
pgplex:mainfrom
zikolach:fix/constraint-include-columns
Open

zikolach wants to merge 1 commit into
pgplex:mainfrom
zikolach:fix/constraint-include-columns

Conversation

@zikolach

Copy link
Copy Markdown
Contributor

Summary

Preserve ordered INCLUDE columns on ordinary and partitioned PRIMARY KEY and UNIQUE constraints.

For example, dump replay currently changes UNIQUE (id, a) INCLUDE (payload) to UNIQUE (id, a). The planner also treats INCLUDE-only changes as no-ops because the constraint IR omits those columns.

  • The inspector reads included columns separately from the uniqueness key.
  • PK/UNIQUE writers retain INCLUDE, with quoted identifiers and deferrability in PostgreSQL syntax order.
  • Constraint comparison detects INCLUDE additions, changes, reordering, and removal.
  • Existing FK recreation and partition attach/detach handling retain the included columns.

Fixes #673. This is an independent, INCLUDE-only follow-up to closed #645 on top of the merged #663 and #668 behavior. It contains no constraint-comment, SQL-loader, or runtime-restoration changes.

Validation

The new regression failed before the fix. Focused native tests and actual CLI dump, psql replay, plan, and apply checks passed on PostgreSQL 14–18. Catalog comparisons verify pg_get_constraintdef and pg_get_indexdef, including nested families, renamed inherited constraints, quoted/ordered included columns, and compatible deferrability. INCLUDE-only migrations with dependent foreign keys produce changes and converge to an empty follow-up plan.

PGSCHEMA_POSTGRES_VERSION=18 go test -count=1 ./internal/diff -run '^TestConstraintInclude'

Fourteen neighboring diff/plan/apply filters and four dump regressions passed on PostgreSQL 18. Relevant IR/plan tests, go vet ./..., build, formatting, and git diff --check passed. sqlc v1.30.0 regeneration is reproducible. The full suite remains for CI.

AI assisted with implementation and independent review. Validation used synthetic schemas on owned local PostgreSQL instances.

@greptile-apps

greptile-apps Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

Preserves ordered INCLUDE columns on primary-key and unique constraints.

  • Constraint dumps keep their ordered INCLUDE columns.
  • Schema plans rebuild constraints when INCLUDE columns change.

No actionable issues found. Tests were inspected but not run during this review.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[PostgreSQL index columns] --> B[Inspect keys and included columns separately]
  B --> C[Constraint.IncludeColumns]
  C --> D[Compare ordered columns]
  C --> E[Render quoted INCLUDE clause]
  D --> F[Rebuild changed constraints]
  E --> G[Dump or migration SQL]
  F --> G
Loading

Reviews (1) · Last reviewed commit: "fix: preserve constraint INCLUDE columns..." · Reviewed by Greptile

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 dependency and ordering gaps cause PostgreSQL to reject partitioned-key INCLUDE changes with leaf-referencing foreign keys and migrations that emit a new self-referencing foreign key before its required INCLUDE primary key.

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

This PR addresses #673 by preserving ordered INCLUDE columns on PRIMARY KEY and UNIQUE constraints throughout pgschema’s inspection, dump, and migration workflow, including partitioned tables.

Changes:

  • Store included columns separately from uniqueness keys.
  • Render quoted INCLUDE lists and detect changes to their contents or order.
  • Add regression coverage for replay, migrations, column changes, and partition attachment/detachment.
File Description
ir/​queries/​queries.sql.go Regenerate query bindings to read included columns.
ir/​queries/​queries.sql Extract ordered included columns from backing indexes.
ir/​ir.go Add included columns to the constraint representation.
ir/​inspector.go Populate included columns during inspection.
internal/​diff/​table.go Preserve INCLUDE in table alterations and column-drop handling.
internal/​diff/​partition.go Compare included columns when matching inherited constraints.
internal/​diff/​partition_test.go Test partition constraint matching and retained INCLUDE metadata.
internal/​diff/​constraint.go Render INCLUDE clauses and detect INCLUDE-only changes.
internal/​diff/​constraint_include_test.go Add catalog-based replay and migration regressions.

Review effort: Balanced

Comment on lines +249 to +250
if !slices.Equal(old.IncludeColumns, new.IncludeColumns) {
return false
Comment thread internal/diff/table.go
Comment on lines +1410 to +1413
// INCLUDE needs table-level syntax after all new columns exist.
if len(constraint.IncludeColumns) > 0 {
continue
}

This branch has not been deployed

No deployments
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.

Preserve INCLUDE columns on PRIMARY KEY and UNIQUE constraints

2 participants