From 18865f4df9ca2369f42f98d53fc0e82e38b8a41d Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Thu, 3 Nov 2022 20:40:21 +0100 Subject: [PATCH] Create FKs properly when attaching table as partition Commit f56f8f8da6af added some code in CloneFkReferencing that's way too lax about a Constraint node it manufactures, not initializing enough struct members -- initially_valid in particular was forgotten. This causes some FKs in partitions added by ALTER TABLE ATTACH PARTITION to be marked as not validated. Set initially_valid true, which fixes the bug. While at it, make the struct initialization more complete. Very similar code was added in two other places by the same commit; make them all follow the same pattern for consistency, though no bugs are apparent there. This bug has never been reported: I only happened to notice while working on commit 614a406b4ff1. The test case that was added there with the improper result is repaired. Backpatch to 12. Discussion: https://postgr.es/m/20221005105523.bhuhkdx4olajboof@alvherre.pgsql --- src/backend/commands/tablecmds.c | 43 ++++++++++++++++++----- src/test/regress/expected/foreign_key.out | 4 +-- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index e5f170a38c..89fc297fe3 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -9858,14 +9858,21 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) mapped_confkey[i] = attmap->attnums[confkey[i] - 1]; fkconstraint = makeNode(Constraint); - /* for now this is all we need */ + fkconstraint->contype = CONSTRAINT_FOREIGN; fkconstraint->conname = NameStr(constrForm->conname); - fkconstraint->fk_upd_action = constrForm->confupdtype; - fkconstraint->fk_del_action = constrForm->confdeltype; fkconstraint->deferrable = constrForm->condeferrable; fkconstraint->initdeferred = constrForm->condeferred; - fkconstraint->initially_valid = true; + fkconstraint->location = -1; + fkconstraint->pktable = NULL; + /* ->fk_attrs determined below */ + fkconstraint->pk_attrs = NIL; fkconstraint->fk_matchtype = constrForm->confmatchtype; + fkconstraint->fk_upd_action = constrForm->confupdtype; + fkconstraint->fk_del_action = constrForm->confdeltype; + fkconstraint->old_conpfeqop = NIL; + fkconstraint->old_pktable_oid = InvalidOid; + fkconstraint->skip_validation = false; + fkconstraint->initially_valid = true; /* set up colnames that are used to generate the constraint name */ for (int i = 0; i < numfks; i++) @@ -10037,11 +10044,21 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) /* No dice. Set up to create our own constraint */ fkconstraint = makeNode(Constraint); - fkconstraint->fk_upd_action = constrForm->confupdtype; - fkconstraint->fk_del_action = constrForm->confdeltype; + fkconstraint->contype = CONSTRAINT_FOREIGN; + /* ->conname determined below */ fkconstraint->deferrable = constrForm->condeferrable; fkconstraint->initdeferred = constrForm->condeferred; + fkconstraint->location = -1; + fkconstraint->pktable = NULL; + /* ->fk_attrs determined below */ + fkconstraint->pk_attrs = NIL; fkconstraint->fk_matchtype = constrForm->confmatchtype; + fkconstraint->fk_upd_action = constrForm->confupdtype; + fkconstraint->fk_del_action = constrForm->confdeltype; + fkconstraint->old_conpfeqop = NIL; + fkconstraint->old_pktable_oid = InvalidOid; + fkconstraint->skip_validation = false; + fkconstraint->initially_valid = true; for (int i = 0; i < numfks; i++) { Form_pg_attribute att; @@ -18019,11 +18036,21 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, * still do), but now we need separate ones of our own. */ fkconstraint = makeNode(Constraint); + fkconstraint->contype = CONSTRAINT_FOREIGN; fkconstraint->conname = pstrdup(NameStr(conform->conname)); - fkconstraint->fk_upd_action = conform->confupdtype; - fkconstraint->fk_del_action = conform->confdeltype; fkconstraint->deferrable = conform->condeferrable; fkconstraint->initdeferred = conform->condeferred; + fkconstraint->location = -1; + fkconstraint->pktable = NULL; + fkconstraint->fk_attrs = NIL; + fkconstraint->pk_attrs = NIL; + fkconstraint->fk_matchtype = conform->confmatchtype; + fkconstraint->fk_upd_action = conform->confupdtype; + fkconstraint->fk_del_action = conform->confdeltype; + fkconstraint->old_conpfeqop = NIL; + fkconstraint->old_pktable_oid = InvalidOid; + fkconstraint->skip_validation = false; + fkconstraint->initially_valid = true; createForeignKeyActionTriggers(partRel, conform->confrelid, fkconstraint, fk->conoid, diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 77ad0faa56..e56e7fbe83 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -1960,7 +1960,7 @@ ORDER BY co.contype, cr.relname, co.conname, p.conname; ----------------+----------------------------+---------+--------------+----------------------------+--------------+---------------- part1_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk part2_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk - part32_self_fk | parted_self_fk_id_abc_fkey | f | f | parted_self_fk_id_abc_fkey | t | parted_self_fk + part32_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk part33_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk part3_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk parted_self_fk | parted_self_fk_id_abc_fkey | f | t | | | parted_self_fk @@ -1989,7 +1989,7 @@ ORDER BY co.contype, cr.relname, co.conname, p.conname; ----------------+----------------------------+---------+--------------+----------------------------+--------------+---------------- part1_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk part2_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk - part32_self_fk | parted_self_fk_id_abc_fkey | f | f | parted_self_fk_id_abc_fkey | t | parted_self_fk + part32_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk part33_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk part3_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk parted_self_fk | parted_self_fk_id_abc_fkey | f | t | | | parted_self_fk