Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Create FKs properly when attaching table as partition
authorAlvaro Herrera <alvherre@alvh.no-ip.org>
Thu, 3 Nov 2022 19:40:21 +0000 (20:40 +0100)
committerAlvaro Herrera <alvherre@alvh.no-ip.org>
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.bhuhkdx4olajboof@alvherre.pgsql

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

index 4d67176b64b0c6dbad10f705e6f736a0c6d708e6..425630228f4c08af189361ce565b1af92a2602bd 100644 (file)
@@ -9465,14 +9465,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++)
@@ -9644,11 +9651,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;
@@ -17401,11 +17418,21 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
         * 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,
index 1c24ab5c2383366b6d8c3cde2115f403084a143d..e235e90d27619f81109027fed09d151d5fd808fd 100644 (file)
@@ -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