Repository navigation
fix: preserve MATCH FULL on foreign keys - #662
Conversation
The inspector did not read pg_constraint.confmatchtype, so dump left MATCH FULL off a foreign key, plan left it off a new foreign key, and a plan from a simple foreign key to a MATCH FULL one reported no changes. Read it into Constraint.MatchFull and write it in the three foreign key writers: the inline and ALTER constraint writer, the REFERENCES clause writer, and the online NOT VALID rewrite. Compare it in constraintsEqual so a change recreates the constraint. GetConstraints selects the same column, so the two queries stay alike.
[Medium impact] The PR appears safe to merge; no actionable issues were found. SummaryPreserves
No actionable issues found. Tests were not run during this review. Diagram%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[PostgreSQL confmatchtype] --> B[Constraint.MatchFull]
B --> C[Compare constraints]
C --> D[Drop and recreate changed key]
B --> E[Inline or standalone SQL]
D --> F[Online add and validate]
E --> G[MATCH FULL preserved]
F --> G
Reviews (1) · Last reviewed commit: "fix: preserve MATCH FULL on foreign keys" · Reviewed by Greptile |
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the changes preserving foreign-key MATCH FULL semantics.
No open issues 🎉
1 file not reviewed
ir/queries/queries.sql.go: Generated file
What changed in this PR
Preserves foreign-key MATCH FULL semantics throughout pgschema’s inspection, dump, and migration workflow, and detects changes between default matching and MATCH FULL.
Changes:
- Reads the match type from PostgreSQL catalogs into
Constraint.MatchFull. - Emits
MATCH FULLin standalone, inline, and online foreign-key DDL and compares it when detecting changes. - Extends regression fixtures for new constraints, constraint replacement, and inline table creation.
| File | Description |
|---|---|
testdata/diff/create_table/add_table_composite_keys/plan.txt |
Updates readable inline-constraint plan. |
testdata/diff/create_table/add_table_composite_keys/plan.sql |
Preserves inline MATCH FULL in plan SQL. |
testdata/diff/create_table/add_table_composite_keys/plan.json |
Updates serialized inline-constraint plan. |
testdata/diff/create_table/add_table_composite_keys/new.sql |
Adds inline MATCH FULL coverage. |
testdata/diff/create_table/add_table_composite_keys/diff.sql |
Updates expected table-creation DDL. |
testdata/diff/create_table/add_fk/plan.txt |
Shows constraint replacement and online addition. |
testdata/diff/create_table/add_fk/plan.sql |
Updates expected online migration SQL. |
testdata/diff/create_table/add_fk/plan.json |
Updates plan steps and source fingerprint. |
testdata/diff/create_table/add_fk/old.sql |
Adds baseline tables and a default-match constraint. |
testdata/diff/create_table/add_fk/new.sql |
Adds desired MATCH FULL constraints. |
testdata/diff/create_table/add_fk/diff.sql |
Records expected constraint addition and replacement. |
ir/queries/queries.sql.go |
Adds generated match-type fields and scanning. |
ir/queries/queries.sql |
Reads confmatchtype in both constraint queries. |
ir/ir.go |
Adds Constraint.MatchFull. |
ir/inspector.go |
Populates the match-type field during inspection. |
internal/plan/rewrite.go |
Preserves MATCH FULL during online rewriting. |
internal/diff/table.go |
Emits MATCH FULL in inline foreign keys. |
internal/diff/constraint.go |
Emits and compares MATCH FULL for constraints. |
Review effort: Balanced
tianzhou
left a comment
There was a problem hiding this comment.
LGTM.
Thanks for the contribution.
A table has
CONSTRAINT c_fk FOREIGN KEY (x, y) REFERENCES p (a, b) MATCH FULL.pgschema dumpprints it withoutMATCH FULL, so a restore accepts rows with one ofx,yNULL, which the original rejects.pgschema planwith a desired file that adds theMATCH FULLforeign key givesADD CONSTRAINT c_fk FOREIGN KEY (x, y) REFERENCES p (a, b) NOT VALID, also without it. If the database already has a plain foreign key namedc_fk, the plan is empty, so apply never addsMATCH FULL. An inline constraint inCREATE TABLEloses it the same way. Every supported Postgres version hasMATCH FULL; I ran this on 18.Cause: the inspector never reads
pg_constraint.confmatchtype. TheGetConstraintsForSchemaselect list (queries.sql#L1397-L1412) has the delete and update rules but not the match type, soir.Constrainthas no field for it and the three foreign key writers print nothing: constraint.go#L55, table.go#L2228 and the online rewrite at rewrite.go#L314.constraintsEqual(constraint.go#L189) does not compare it, so the two keys look equal. It is the same gap #589 had for theON DELETE SET NULL (col)list, fixed in #590.Fix: the query returns
confmatchtype = 'f'asmatch_fulland the inspector stores it inConstraint.MatchFull. Each writer printsMATCH FULLright after the referenced column list, where the grammar puts it.constraintsEqualcompares it, so a change drops and re-adds the constraint, as forON DELETE.MATCH SIMPLEis the default andMATCH PARTIALis not implemented by Postgres.queries.sql.gois the output ofsqlc generate(v1.30.0): rerunning it changes nothing.GetConstraintsgets the same column, as in #590, so the two queries stay alike.Tests: as in #590, I added the cases to existing directories.
create_table/add_fkgets a newMATCH FULLkey on an existing table (online rewrite) and a plain key that becomesMATCH FULL.create_table/add_table_composite_keysgets the inline form. The plan files were regenerated with--generate.With the fix reverted and the tests kept, each command fails 2 of 29 subtests (
add_fk,add_table_composite_keys). With the fix both pass 29 of 29. Reverting one hunk at a time fails a test for each: the inspector line and the query (both cases),constraintsEqual(add_fk), the inline writer (add_table_composite_keys), theREFERENCESwriter (add_fk, diff test) and the rewrite (add_fk, plan test).go test -count=1 ./ir/... ./internal/diff ./internal/planand the five dump tests with foreign keys pass, on the default embedded Postgres 18 only.gofmt -l,go veton those packages andgit diff --checkare clean.Overlap checked: searched open and closed issues and PRs for
"MATCH FULL",confmatchtype,MATCH SIMPLE,foreign key matchand listed the open PRs. None mentions the match type. #645, #660 and #622 edit other functions intable.goandrewrite.go(and #645 the top ofir.Constraint);git merge-treeagainst each of those heads merges cleanly with this change. #637 already conflicts with main inir/inspector.go, in the same three places with or without this change.