Create FKs properly when attaching table as partition
authorAlvaro Herrera <[email protected]>
Thu, 3 Nov 2022 19:40:21 +0000 (20:40 +0100)
committerAlvaro Herrera <[email protected]>
Thu, 3 Nov 2022 19:40:21 +0000 (20:40 +0100)
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[email protected]

src/backend/commands/tablecmds.c
src/test/regress/expected/foreign_key.out

index 140ecb0cbfad344a2b3f94c978e52dbad56dcabe..6007e10730a119d37f3a2a1f7492d0d1a45a1913 100644 (file)
@@ -10092,14 +10092,22 @@ 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->fk_del_set_cols = NIL;
+       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++)
@@ -10317,11 +10325,22 @@ 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->fk_del_set_cols = NIL;
+       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;
@@ -18517,11 +18536,22 @@ 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->fk_del_set_cols = NIL;
+       fkconstraint->old_conpfeqop = NIL;
+       fkconstraint->old_pktable_oid = InvalidOid;
+       fkconstraint->skip_validation = false;
+       fkconstraint->initially_valid = true;
 
        createForeignKeyActionTriggers(partRel, conform->confrelid,
                                       fkconstraint, fk->conoid,
index ae5276ff1775b55ce79f38e00572b1101d655a46..2f9c083539e5eeb84a672c7b161124c984f3d1b5 100644 (file)
@@ -2031,7 +2031,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
@@ -2060,7 +2060,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