On Thu, 5 Sep 2024 00:57:28 +0200
Jehan-Guillaume de Rorthais <j...@dalibo.com> wrote:

> On Mon, 2 Sep 2024 23:01:47 +0200
> Jehan-Guillaume de Rorthais <j...@dalibo.com> wrote:
> 
> […]
> 
> >   My proposal was to clean everything related to the old FK and use some
> >   existing code path to create a fresh and cleaner one. This requires some
> >   refactoring in existing code, but we would win a common path of code
> > between create/attach/detach, a cleaner catalog and easier code maintenance.
> > 
> >   I've finally been able to write a PoC that implement this by calling
> >   addFkRecurseReferenced() from DetachPartitionFinalize(). I can't join
> >   it here because it is currently an ugly draft and I still have some work
> >   to do. But I would really like to have a little more time (one or two
> > days) to explore this avenue further before you commit yours, if you don't
> > mind? Or maybe you already have considered this avenue and rejected it?  
> 
> Please, find in attachment a patch implementing this idea.

Please, find in attachment a set of patch based on the previous one.

v3-0001-Add-tests-about-FK-between-partitionned-tables.patch:

  This patch implement tests triggering the bugs discussed. Based on Michael
  advice, I added one level sub-partitioning to stress test the recursive code
  and some queries checking on the catalog objects.

v3-0002-Rework-foreign-key-mangling-during-ATTACH-DETACH.patch:

  The main patch, similar to v2 in my previous patch with more comments
  added/restored. I added some more explanations in the commit message about
  the refactoring itself, making addFkRecurseReferencing() and
  addFkRecurseReferenced() having the same logic.

v3-0003-Use-addFkConstraint-in-addFkRecurseReferencing.patch

  A new patch refactoring the constraint creation in addFkRecurseReferencing()
  to use the new addFkConstraint() function.

v3-0004-Use-addFkConstraint-in-CloneFkReferencing.patch

  A new patch refactoring the constraint creation in CloneFkReferencing()
  to use the new addFkConstraint() function.

TODO:

* I hadn't time to study last Tender Wang comment here:
  
https://postgr.es/m/CAHewXNkuU2V7GfgFkwd265RJ99%2BBfJueNEZhrHSk39o3thqxNA%40mail.gmail.com
* I still think we should split v3-0002 in two different patch…
* backporting…

Regards,
>From 250de46d4c9dbdfc9d6ae44288f863a4226edd1e Mon Sep 17 00:00:00 2001
From: Jehan-Guillaume de Rorthais <j...@dalibo.com>
Date: Mon, 23 Sep 2024 18:50:20 +0200
Subject: [PATCH v3 1/4] Add tests about FK between partitionned tables

These tests cover two reported bugs related to FK
between two partitionned tables.

co-author : Alvaro Herrera <alvhe...@alvh.no-ip.org>
co-author : Jehan-Guillaume de Rorthais <j...@dalibo.com>
co-author : Tender Wang <tndrw...@gmail.com>

Discussion: https://postgr.es/m/20230420144344.40744130@karst
Discussion: https://postgr.es/m/20230705233028.2f554f73@karst
---
 src/test/regress/expected/foreign_key.out | 266 ++++++++++++++++++++++
 src/test/regress/sql/foreign_key.sql      | 101 ++++++++
 2 files changed, 367 insertions(+)

diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 8c04a24b37..da67bbeb4f 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -2934,3 +2934,269 @@ DETAIL:  drop cascades to table fkpart11.pk
 drop cascades to table fkpart11.fk_parted
 drop cascades to table fkpart11.fk_another
 drop cascades to function fkpart11.print_row()
+-- When a table is attached as partition to a partitioned table that has
+-- a foreign key to another partitioned table, it acquires a clone of the
+-- FK.  Upon detach, this clone is not removed, but instead becomes an
+-- independent FK.  If it then attaches to the partitioned table again,
+-- the FK from the parent "takes over" ownership of the independent FK rather
+-- than creating a separate one.
+CREATE SCHEMA fkpart12
+  CREATE TABLE fk_p ( id int, jd int, PRIMARY KEY(id, jd)) PARTITION BY list (id)
+  CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1) PARTITION BY list (jd)
+  CREATE TABLE fk_p_1_1 PARTITION OF fk_p_1 FOR VALUES IN (1)
+  CREATE TABLE fk_p_1_2 PARTITION OF fk_p_1 FOR VALUES IN (2)
+  CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2) PARTITION BY list (jd)
+  CREATE TABLE fk_p_2_1 PARTITION OF fk_p_2 FOR VALUES IN (1)
+  CREATE TABLE fk_p_2_2 PARTITION OF fk_p_2 FOR VALUES IN (2)
+  CREATE TABLE fk_r_1 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL)
+  CREATE TABLE fk_r_2 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL)
+  CREATE TABLE fk_r   ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL,
+       FOREIGN KEY (p_id, p_jd) REFERENCES fk_p (id, jd)
+  ) PARTITION BY list (id);
+SET search_path TO fkpart12;
+CREATE OR REPLACE FUNCTION
+fkpart12_constraints(OUT conname name, OUT conrel regclass,
+                   OUT confrelid regclass, OUT conparent name) RETURNS SETOF record AS $$
+  WITH RECURSIVE r AS (
+    SELECT oid, conname, conrelid, confrelid, NULL::name AS conparent
+    FROM pg_constraint
+    WHERE conrelid = 'fk_r'::regclass
+      AND contype = 'f'
+      AND conparentid = 0
+    UNION ALL
+    SELECT c.oid, c.conname, c.conrelid, c.confrelid, r.conname
+    FROM pg_constraint c
+    JOIN r ON c.conparentid = r.oid
+  )
+  SELECT conname, conrelid::regclass, confrelid::regclass, conparent
+  FROM r
+  ORDER BY oid, conname, confrelid, conparent;
+$$
+LANGUAGE SQL;
+CREATE OR REPLACE FUNCTION
+fkpart12_triggers(OUT conname name, OUT conparent name,
+                   OUT tgfoid regproc, OUT tgfparent regproc) RETURNS SETOF record AS $$
+
+  WITH RECURSIVE r AS (
+    SELECT t.oid, t.tgfoid::regproc, c.conname, NULL::name AS conparent, NULL::regproc AS tgfparent
+    FROM pg_trigger t
+    JOIN pg_constraint c ON c.oid = t.tgconstraint
+    WHERE c.conrelid = 'fk_r'::regclass
+      AND c.contype = 'f'
+      AND t.tgparentid = 0
+    UNION ALL
+    SELECT t2.oid, t2.tgfoid::regproc, c2.conname, c3.conname, r.tgfoid::regproc AS tgfparent
+    FROM pg_trigger t2
+    JOIN pg_constraint c2 ON c2.oid = t2.tgconstraint
+    LEFT JOIN pg_constraint c3 ON c3.oid = c2.conparentid
+    JOIN r ON r.oid = t2.tgparentid
+  )
+  SELECT conname, conparent, tgfoid, tgfparent
+  FROM r
+  ORDER BY conname, conparent, tgfoid, tgfparent;
+$$
+LANGUAGE SQL;
+SELECT * FROM fkpart12_constraints();
+       conname        | conrel | confrelid |      conparent       
+----------------------+--------+-----------+----------------------
+ fk_r_p_id_p_jd_fkey  | fk_r   | fk_p      | 
+ fk_r_p_id_p_jd_fkey1 | fk_r   | fk_p_1    | fk_r_p_id_p_jd_fkey
+ fk_r_p_id_p_jd_fkey2 | fk_r   | fk_p_1_1  | fk_r_p_id_p_jd_fkey1
+ fk_r_p_id_p_jd_fkey3 | fk_r   | fk_p_1_2  | fk_r_p_id_p_jd_fkey1
+ fk_r_p_id_p_jd_fkey4 | fk_r   | fk_p_2    | fk_r_p_id_p_jd_fkey
+ fk_r_p_id_p_jd_fkey5 | fk_r   | fk_p_2_1  | fk_r_p_id_p_jd_fkey4
+ fk_r_p_id_p_jd_fkey6 | fk_r   | fk_p_2_2  | fk_r_p_id_p_jd_fkey4
+(7 rows)
+
+SELECT * FROM fkpart12_triggers();
+       conname        |      conparent       |         tgfoid         |       tgfparent        
+----------------------+----------------------+------------------------+------------------------
+ fk_r_p_id_p_jd_fkey  |                      | "RI_FKey_check_ins"    | 
+ fk_r_p_id_p_jd_fkey  |                      | "RI_FKey_check_upd"    | 
+ fk_r_p_id_p_jd_fkey  |                      | "RI_FKey_noaction_del" | 
+ fk_r_p_id_p_jd_fkey  |                      | "RI_FKey_noaction_upd" | 
+ fk_r_p_id_p_jd_fkey1 | fk_r_p_id_p_jd_fkey  | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey1 | fk_r_p_id_p_jd_fkey  | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey2 | fk_r_p_id_p_jd_fkey1 | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey2 | fk_r_p_id_p_jd_fkey1 | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey3 | fk_r_p_id_p_jd_fkey1 | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey3 | fk_r_p_id_p_jd_fkey1 | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey4 | fk_r_p_id_p_jd_fkey  | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey4 | fk_r_p_id_p_jd_fkey  | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey5 | fk_r_p_id_p_jd_fkey4 | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey5 | fk_r_p_id_p_jd_fkey4 | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey6 | fk_r_p_id_p_jd_fkey4 | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey6 | fk_r_p_id_p_jd_fkey4 | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+(16 rows)
+
+INSERT INTO fk_p VALUES (1, 1);
+ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1);
+ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2);
+\d fk_r_1
+              Table "fkpart12.fk_r_1"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ id     | integer |           | not null | 
+ p_id   | integer |           | not null | 
+ p_jd   | integer |           | not null | 
+Partition of: fk_r FOR VALUES IN (1)
+Indexes:
+    "fk_r_1_pkey" PRIMARY KEY, btree (id)
+Foreign-key constraints:
+    TABLE "fk_r" CONSTRAINT "fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd)
+
+SELECT * FROM fkpart12_constraints();
+       conname        | conrel | confrelid |      conparent       
+----------------------+--------+-----------+----------------------
+ fk_r_p_id_p_jd_fkey  | fk_r   | fk_p      | 
+ fk_r_p_id_p_jd_fkey1 | fk_r   | fk_p_1    | fk_r_p_id_p_jd_fkey
+ fk_r_p_id_p_jd_fkey2 | fk_r   | fk_p_1_1  | fk_r_p_id_p_jd_fkey1
+ fk_r_p_id_p_jd_fkey3 | fk_r   | fk_p_1_2  | fk_r_p_id_p_jd_fkey1
+ fk_r_p_id_p_jd_fkey4 | fk_r   | fk_p_2    | fk_r_p_id_p_jd_fkey
+ fk_r_p_id_p_jd_fkey5 | fk_r   | fk_p_2_1  | fk_r_p_id_p_jd_fkey4
+ fk_r_p_id_p_jd_fkey6 | fk_r   | fk_p_2_2  | fk_r_p_id_p_jd_fkey4
+ fk_r_p_id_p_jd_fkey  | fk_r_1 | fk_p      | fk_r_p_id_p_jd_fkey
+ fk_r_p_id_p_jd_fkey  | fk_r_2 | fk_p      | fk_r_p_id_p_jd_fkey
+(9 rows)
+
+SELECT * FROM fkpart12_triggers();
+       conname        |      conparent       |         tgfoid         |       tgfparent        
+----------------------+----------------------+------------------------+------------------------
+ fk_r_p_id_p_jd_fkey  | fk_r_p_id_p_jd_fkey  | "RI_FKey_check_ins"    | "RI_FKey_check_ins"
+ fk_r_p_id_p_jd_fkey  | fk_r_p_id_p_jd_fkey  | "RI_FKey_check_ins"    | "RI_FKey_check_ins"
+ fk_r_p_id_p_jd_fkey  | fk_r_p_id_p_jd_fkey  | "RI_FKey_check_upd"    | "RI_FKey_check_upd"
+ fk_r_p_id_p_jd_fkey  | fk_r_p_id_p_jd_fkey  | "RI_FKey_check_upd"    | "RI_FKey_check_upd"
+ fk_r_p_id_p_jd_fkey  |                      | "RI_FKey_check_ins"    | 
+ fk_r_p_id_p_jd_fkey  |                      | "RI_FKey_check_upd"    | 
+ fk_r_p_id_p_jd_fkey  |                      | "RI_FKey_noaction_del" | 
+ fk_r_p_id_p_jd_fkey  |                      | "RI_FKey_noaction_upd" | 
+ fk_r_p_id_p_jd_fkey1 | fk_r_p_id_p_jd_fkey  | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey1 | fk_r_p_id_p_jd_fkey  | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey2 | fk_r_p_id_p_jd_fkey1 | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey2 | fk_r_p_id_p_jd_fkey1 | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey3 | fk_r_p_id_p_jd_fkey1 | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey3 | fk_r_p_id_p_jd_fkey1 | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey4 | fk_r_p_id_p_jd_fkey  | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey4 | fk_r_p_id_p_jd_fkey  | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey5 | fk_r_p_id_p_jd_fkey4 | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey5 | fk_r_p_id_p_jd_fkey4 | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey6 | fk_r_p_id_p_jd_fkey4 | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey6 | fk_r_p_id_p_jd_fkey4 | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+(20 rows)
+
+INSERT INTO fk_r VALUES (1, 1, 1);
+ALTER TABLE fk_r DETACH PARTITION fk_r_1;
+\d fk_r_1
+              Table "fkpart12.fk_r_1"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ id     | integer |           | not null | 
+ p_id   | integer |           | not null | 
+ p_jd   | integer |           | not null | 
+Indexes:
+    "fk_r_1_pkey" PRIMARY KEY, btree (id)
+Foreign-key constraints:
+    "fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd)
+
+SELECT * FROM fkpart12_constraints();
+       conname        | conrel | confrelid |      conparent       
+----------------------+--------+-----------+----------------------
+ fk_r_p_id_p_jd_fkey  | fk_r   | fk_p      | 
+ fk_r_p_id_p_jd_fkey1 | fk_r   | fk_p_1    | fk_r_p_id_p_jd_fkey
+ fk_r_p_id_p_jd_fkey2 | fk_r   | fk_p_1_1  | fk_r_p_id_p_jd_fkey1
+ fk_r_p_id_p_jd_fkey3 | fk_r   | fk_p_1_2  | fk_r_p_id_p_jd_fkey1
+ fk_r_p_id_p_jd_fkey4 | fk_r   | fk_p_2    | fk_r_p_id_p_jd_fkey
+ fk_r_p_id_p_jd_fkey5 | fk_r   | fk_p_2_1  | fk_r_p_id_p_jd_fkey4
+ fk_r_p_id_p_jd_fkey6 | fk_r   | fk_p_2_2  | fk_r_p_id_p_jd_fkey4
+ fk_r_p_id_p_jd_fkey  | fk_r_2 | fk_p      | fk_r_p_id_p_jd_fkey
+(8 rows)
+
+SELECT * FROM fkpart12_triggers();
+       conname        |      conparent       |         tgfoid         |       tgfparent        
+----------------------+----------------------+------------------------+------------------------
+ fk_r_p_id_p_jd_fkey  | fk_r_p_id_p_jd_fkey  | "RI_FKey_check_ins"    | "RI_FKey_check_ins"
+ fk_r_p_id_p_jd_fkey  | fk_r_p_id_p_jd_fkey  | "RI_FKey_check_upd"    | "RI_FKey_check_upd"
+ fk_r_p_id_p_jd_fkey  |                      | "RI_FKey_check_ins"    | 
+ fk_r_p_id_p_jd_fkey  |                      | "RI_FKey_check_upd"    | 
+ fk_r_p_id_p_jd_fkey  |                      | "RI_FKey_noaction_del" | 
+ fk_r_p_id_p_jd_fkey  |                      | "RI_FKey_noaction_upd" | 
+ fk_r_p_id_p_jd_fkey1 | fk_r_p_id_p_jd_fkey  | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey1 | fk_r_p_id_p_jd_fkey  | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey2 | fk_r_p_id_p_jd_fkey1 | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey2 | fk_r_p_id_p_jd_fkey1 | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey3 | fk_r_p_id_p_jd_fkey1 | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey3 | fk_r_p_id_p_jd_fkey1 | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey4 | fk_r_p_id_p_jd_fkey  | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey4 | fk_r_p_id_p_jd_fkey  | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey5 | fk_r_p_id_p_jd_fkey4 | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey5 | fk_r_p_id_p_jd_fkey4 | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey6 | fk_r_p_id_p_jd_fkey4 | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey6 | fk_r_p_id_p_jd_fkey4 | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+(18 rows)
+
+INSERT INTO fk_r_1 VALUES (2, 1, 2); -- fails as EXPECTED
+ERROR:  insert or update on table "fk_r_1" violates foreign key constraint "fk_r_p_id_p_jd_fkey"
+DETAIL:  Key (p_id, p_jd)=(1, 2) is not present in table "fk_p".
+DELETE FROM fk_p; -- should fail but was buggy
+ERROR:  update or delete on table "fk_p_1_1" violates foreign key constraint "fk_r_1_p_id_p_jd_fkey1" on table "fk_r_1"
+DETAIL:  Key (id, jd)=(1, 1) is still referenced from table "fk_r_1".
+ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1);
+\d fk_r_1
+              Table "fkpart12.fk_r_1"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ id     | integer |           | not null | 
+ p_id   | integer |           | not null | 
+ p_jd   | integer |           | not null | 
+Partition of: fk_r FOR VALUES IN (1)
+Indexes:
+    "fk_r_1_pkey" PRIMARY KEY, btree (id)
+Foreign-key constraints:
+    TABLE "fk_r" CONSTRAINT "fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd)
+
+SELECT * FROM fkpart12_constraints();
+       conname        | conrel | confrelid |      conparent       
+----------------------+--------+-----------+----------------------
+ fk_r_p_id_p_jd_fkey  | fk_r   | fk_p      | 
+ fk_r_p_id_p_jd_fkey1 | fk_r   | fk_p_1    | fk_r_p_id_p_jd_fkey
+ fk_r_p_id_p_jd_fkey2 | fk_r   | fk_p_1_1  | fk_r_p_id_p_jd_fkey1
+ fk_r_p_id_p_jd_fkey3 | fk_r   | fk_p_1_2  | fk_r_p_id_p_jd_fkey1
+ fk_r_p_id_p_jd_fkey4 | fk_r   | fk_p_2    | fk_r_p_id_p_jd_fkey
+ fk_r_p_id_p_jd_fkey5 | fk_r   | fk_p_2_1  | fk_r_p_id_p_jd_fkey4
+ fk_r_p_id_p_jd_fkey6 | fk_r   | fk_p_2_2  | fk_r_p_id_p_jd_fkey4
+ fk_r_p_id_p_jd_fkey  | fk_r_1 | fk_p      | fk_r_p_id_p_jd_fkey
+ fk_r_p_id_p_jd_fkey  | fk_r_2 | fk_p      | fk_r_p_id_p_jd_fkey
+(9 rows)
+
+SELECT * FROM fkpart12_triggers();
+       conname        |      conparent       |         tgfoid         |       tgfparent        
+----------------------+----------------------+------------------------+------------------------
+ fk_r_p_id_p_jd_fkey  | fk_r_p_id_p_jd_fkey  | "RI_FKey_check_ins"    | "RI_FKey_check_ins"
+ fk_r_p_id_p_jd_fkey  | fk_r_p_id_p_jd_fkey  | "RI_FKey_check_ins"    | "RI_FKey_check_ins"
+ fk_r_p_id_p_jd_fkey  | fk_r_p_id_p_jd_fkey  | "RI_FKey_check_upd"    | "RI_FKey_check_upd"
+ fk_r_p_id_p_jd_fkey  | fk_r_p_id_p_jd_fkey  | "RI_FKey_check_upd"    | "RI_FKey_check_upd"
+ fk_r_p_id_p_jd_fkey  |                      | "RI_FKey_check_ins"    | 
+ fk_r_p_id_p_jd_fkey  |                      | "RI_FKey_check_upd"    | 
+ fk_r_p_id_p_jd_fkey  |                      | "RI_FKey_noaction_del" | 
+ fk_r_p_id_p_jd_fkey  |                      | "RI_FKey_noaction_upd" | 
+ fk_r_p_id_p_jd_fkey1 | fk_r_p_id_p_jd_fkey  | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey1 | fk_r_p_id_p_jd_fkey  | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey2 | fk_r_p_id_p_jd_fkey1 | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey2 | fk_r_p_id_p_jd_fkey1 | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey3 | fk_r_p_id_p_jd_fkey1 | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey3 | fk_r_p_id_p_jd_fkey1 | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey4 | fk_r_p_id_p_jd_fkey  | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey4 | fk_r_p_id_p_jd_fkey  | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey5 | fk_r_p_id_p_jd_fkey4 | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey5 | fk_r_p_id_p_jd_fkey4 | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+ fk_r_p_id_p_jd_fkey6 | fk_r_p_id_p_jd_fkey4 | "RI_FKey_noaction_del" | "RI_FKey_noaction_del"
+ fk_r_p_id_p_jd_fkey6 | fk_r_p_id_p_jd_fkey4 | "RI_FKey_noaction_upd" | "RI_FKey_noaction_upd"
+(20 rows)
+
+DELETE FROM fk_p; -- should fail but was buggy
+ERROR:  update or delete on table "fk_p_1_1" violates foreign key constraint "fk_r_p_id_p_jd_fkey2" on table "fk_r"
+DETAIL:  Key (id, jd)=(1, 1) is still referenced from table "fk_r".
+SET client_min_messages TO warning;
+DROP SCHEMA fkpart12 CASCADE;
+RESET client_min_messages;
+RESET search_path;
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index d1aac5357f..e96ab5c703 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -2086,3 +2086,104 @@ UPDATE fkpart11.pk SET a = 3 WHERE a = 4;
 UPDATE fkpart11.pk SET a = 1 WHERE a = 2;
 
 DROP SCHEMA fkpart11 CASCADE;
+
+-- When a table is attached as partition to a partitioned table that has
+-- a foreign key to another partitioned table, it acquires a clone of the
+-- FK.  Upon detach, this clone is not removed, but instead becomes an
+-- independent FK.  If it then attaches to the partitioned table again,
+-- the FK from the parent "takes over" ownership of the independent FK rather
+-- than creating a separate one.
+CREATE SCHEMA fkpart12
+  CREATE TABLE fk_p ( id int, jd int, PRIMARY KEY(id, jd)) PARTITION BY list (id)
+  CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1) PARTITION BY list (jd)
+  CREATE TABLE fk_p_1_1 PARTITION OF fk_p_1 FOR VALUES IN (1)
+  CREATE TABLE fk_p_1_2 PARTITION OF fk_p_1 FOR VALUES IN (2)
+  CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2) PARTITION BY list (jd)
+  CREATE TABLE fk_p_2_1 PARTITION OF fk_p_2 FOR VALUES IN (1)
+  CREATE TABLE fk_p_2_2 PARTITION OF fk_p_2 FOR VALUES IN (2)
+  CREATE TABLE fk_r_1 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL)
+  CREATE TABLE fk_r_2 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL)
+  CREATE TABLE fk_r   ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL,
+       FOREIGN KEY (p_id, p_jd) REFERENCES fk_p (id, jd)
+  ) PARTITION BY list (id);
+SET search_path TO fkpart12;
+
+CREATE OR REPLACE FUNCTION
+fkpart12_constraints(OUT conname name, OUT conrel regclass,
+                   OUT confrelid regclass, OUT conparent name) RETURNS SETOF record AS $$
+  WITH RECURSIVE r AS (
+    SELECT oid, conname, conrelid, confrelid, NULL::name AS conparent
+    FROM pg_constraint
+    WHERE conrelid = 'fk_r'::regclass
+      AND contype = 'f'
+      AND conparentid = 0
+    UNION ALL
+    SELECT c.oid, c.conname, c.conrelid, c.confrelid, r.conname
+    FROM pg_constraint c
+    JOIN r ON c.conparentid = r.oid
+  )
+  SELECT conname, conrelid::regclass, confrelid::regclass, conparent
+  FROM r
+  ORDER BY oid, conname, confrelid, conparent;
+$$
+LANGUAGE SQL;
+
+CREATE OR REPLACE FUNCTION
+fkpart12_triggers(OUT conname name, OUT conparent name,
+                   OUT tgfoid regproc, OUT tgfparent regproc) RETURNS SETOF record AS $$
+
+  WITH RECURSIVE r AS (
+    SELECT t.oid, t.tgfoid::regproc, c.conname, NULL::name AS conparent, NULL::regproc AS tgfparent
+    FROM pg_trigger t
+    JOIN pg_constraint c ON c.oid = t.tgconstraint
+    WHERE c.conrelid = 'fk_r'::regclass
+      AND c.contype = 'f'
+      AND t.tgparentid = 0
+    UNION ALL
+    SELECT t2.oid, t2.tgfoid::regproc, c2.conname, c3.conname, r.tgfoid::regproc AS tgfparent
+    FROM pg_trigger t2
+    JOIN pg_constraint c2 ON c2.oid = t2.tgconstraint
+    LEFT JOIN pg_constraint c3 ON c3.oid = c2.conparentid
+    JOIN r ON r.oid = t2.tgparentid
+  )
+  SELECT conname, conparent, tgfoid, tgfparent
+  FROM r
+  ORDER BY conname, conparent, tgfoid, tgfparent;
+$$
+LANGUAGE SQL;
+
+SELECT * FROM fkpart12_constraints();
+SELECT * FROM fkpart12_triggers();
+
+INSERT INTO fk_p VALUES (1, 1);
+
+ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1);
+ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2);
+
+\d fk_r_1
+SELECT * FROM fkpart12_constraints();
+SELECT * FROM fkpart12_triggers();
+
+INSERT INTO fk_r VALUES (1, 1, 1);
+
+ALTER TABLE fk_r DETACH PARTITION fk_r_1;
+
+\d fk_r_1
+SELECT * FROM fkpart12_constraints();
+SELECT * FROM fkpart12_triggers();
+
+INSERT INTO fk_r_1 VALUES (2, 1, 2); -- fails as EXPECTED
+DELETE FROM fk_p; -- should fail but was buggy
+
+ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1);
+
+\d fk_r_1
+SELECT * FROM fkpart12_constraints();
+SELECT * FROM fkpart12_triggers();
+
+DELETE FROM fk_p; -- should fail but was buggy
+
+SET client_min_messages TO warning;
+DROP SCHEMA fkpart12 CASCADE;
+RESET client_min_messages;
+RESET search_path;
-- 
2.46.0

>From b1a81adac1a38f66d4ee6bc03846858eae26e3dd Mon Sep 17 00:00:00 2001
From: Jehan-Guillaume de Rorthais <j...@dalibo.com>
Date: Mon, 23 Sep 2024 18:53:12 +0200
Subject: [PATCH v3 2/4] Rework foreign key mangling during ATTACH/DETACH

... when the reference table is partitioned.

It turns out that the catalog representation we chose for foreign keys
connecting partitioned tables is suboptimal, particularly in the sense
that a standalone table has a different way to represent it when
referencing a partitioned table, than when the same table becomes a
partition (and vice versa).  This difference means we need to spawn
additional catalog rows on detach, and remove them on attach.

As a very obvious symptom, we were missing action triggers after
detach, which means that you could update/delete rows from the
referenced partitioned table that still had referencing rows at the
other side previously detached, and fail to throw the required errors.
This means existing FKs might have rows that break relational
integrity.

To fix this bug, this patch rework addFkRecurseReferenced() so it
can be called from DetachPartitionFinalize(), so we can keep this
logic in a single function. The addFkRecurseReferenced() rework
leads to create addFkConstraint(), but it seems cleaner as
addFkRecurseReferenced() and addFkRecurseReferencing() have the same
logic now. Some other code factoring using addFkConstraint() are
added in next commits.

Another possible problem is that trying to reattach a table
that had been detached would fail indicating that internal triggers
cannot be found, which from the user's point of view is nonsensical.

We might want to rethink the representation in the future to avoid this
messiness, but the code now seems to do what's required to make the
constraints operate correctly.

 co-author: Alvaro Herrera <alvhe...@alvh.no-ip.org>
 co-author: Jehan-Guillaume de Rorthais <j...@dalibo.com>
 co-author: Tender Wang <tndrw...@gmail.com>

Reported-by: Guillaume Lelarge <guilla...@lelarge.info>
Reported-by: Jehan-Guillaume de Rorthais <j...@dalibo.com>
Reported-by: Thomas Baehler (SBB CFF FFS) <thomas.baehl...@sbb.ch>

Discussion: https://postgr.es/m/20230420144344.40744130@karst
Discussion: https://postgr.es/m/20230705233028.2f554f73@karst
Discussion: https://postgr.es/m/gvap278mb02787e7134fd691861635a8bc9...@gvap278mb0278.chep278.prod.outlook.com
Discussion: https://postgr.es/m/18541-628a61bc267cd...@postgresql.org
---
 src/backend/commands/tablecmds.c | 376 ++++++++++++++++++++++++-------
 1 file changed, 295 insertions(+), 81 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d27e6cf345..f7e0b8fa87 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -508,7 +508,13 @@ static ObjectAddress ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *
 											   Relation rel, Constraint *fkconstraint,
 											   bool recurse, bool recursing,
 											   LOCKMODE lockmode);
-static ObjectAddress addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint,
+static ObjectAddress addFkConstraint(Constraint *fkconstraint, Relation rel,
+									 Relation pkrel, Oid indexOid, Oid parentConstr,
+									 int numfks, int16 *pkattnum, int16 *fkattnum,
+									 Oid *pfeqoperators, Oid *ppeqoperators,
+									 Oid *ffeqoperators, int numfkdelsetcols,
+									 int16 *fkdelsetcols, bool with_period);
+static void addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint,
 											Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr,
 											int numfks, int16 *pkattnum, int16 *fkattnum,
 											Oid *pfeqoperators, Oid *ppeqoperators, Oid *ffeqoperators,
@@ -652,9 +658,11 @@ static void DropClonedTriggersFromPartition(Oid partitionId);
 static ObjectAddress ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab,
 										   Relation rel, RangeVar *name,
 										   bool concurrent);
-static void DetachPartitionFinalize(Relation rel, Relation partRel,
-									bool concurrent, Oid defaultPartOid);
-static ObjectAddress ATExecDetachPartitionFinalize(Relation rel, RangeVar *name);
+static void DetachPartitionFinalize(List **wqueue, Relation rel,
+									Relation partRel, bool concurrent,
+									Oid defaultPartOid);
+static ObjectAddress ATExecDetachPartitionFinalize(List **wqueue, Relation rel,
+												   RangeVar *name);
 static ObjectAddress ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx,
 											  RangeVar *name);
 static void validatePartitionedIndex(Relation partedIdx, Relation partedTbl);
@@ -5515,7 +5523,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
 											((PartitionCmd *) cmd->def)->concurrent);
 			break;
 		case AT_DetachPartitionFinalize:
-			address = ATExecDetachPartitionFinalize(rel, ((PartitionCmd *) cmd->def)->name);
+			address = ATExecDetachPartitionFinalize(wqueue, rel, ((PartitionCmd *) cmd->def)->name);
 			break;
 		default:				/* oops */
 			elog(ERROR, "unrecognized alter table type: %d",
@@ -10061,9 +10069,22 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	 * Create all the constraint and trigger objects, recursing to partitions
 	 * as necessary.  First handle the referenced side.
 	 */
-	address = addFkRecurseReferenced(wqueue, fkconstraint, rel, pkrel,
+	address = addFkConstraint(fkconstraint, rel, pkrel,
+							  indexOid,
+							  InvalidOid,	/* no parent constraint */
+							  numfks,
+							  pkattnum,
+							  fkattnum,
+							  pfeqoperators,
+							  ppeqoperators,
+							  ffeqoperators,
+							  numfkdelsetcols,
+							  fkdelsetcols,
+							  with_period);
+
+	addFkRecurseReferenced(wqueue, fkconstraint, rel, pkrel,
 									 indexOid,
-									 InvalidOid,	/* no parent constraint */
+									 address.objectId,
 									 numfks,
 									 pkattnum,
 									 fkattnum,
@@ -10137,47 +10158,33 @@ validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums,
 }
 
 /*
- * addFkRecurseReferenced
- *		subroutine for ATAddForeignKeyConstraint; recurses on the referenced
- *		side of the constraint
+ * addFkConstraint
+ *		Add a new Foreign Key constraint.
  *
- * Create pg_constraint rows for the referenced side of the constraint,
- * referencing the parent of the referencing side; also create action triggers
- * on leaf partitions.  If the table is partitioned, recurse to handle each
- * partition.
+ * Creates a pg_constraint rows referencing a parent if given.
  *
- * wqueue is the ALTER TABLE work queue; can be NULL when not running as part
- * of an ALTER TABLE sequence.
- * fkconstraint is the constraint being added.
- * rel is the root referencing relation.
- * pkrel is the referenced relation; might be a partition, if recursing.
- * indexOid is the OID of the index (on pkrel) implementing this constraint.
- * parentConstr is the OID of a parent constraint; InvalidOid if this is a
- * top-level constraint.
- * numfks is the number of columns in the foreign key
- * pkattnum is the attnum array of referenced attributes.
- * fkattnum is the attnum array of referencing attributes.
- * numfkdelsetcols is the number of columns in the ON DELETE SET NULL/DEFAULT
- *      (...) clause
- * fkdelsetcols is the attnum array of the columns in the ON DELETE SET
- *      NULL/DEFAULT clause
- * pf/pp/ffeqoperators are OID array of operators between columns.
- * old_check_ok signals that this constraint replaces an existing one that
- * was already validated (thus this one doesn't need validation).
- * parentDelTrigger and parentUpdTrigger, when being recursively called on
- * a partition, are the OIDs of the parent action triggers for DELETE and
- * UPDATE respectively.
+ * * fkconstraint is the constraint being added.
+ * * rel is the root referencing relation.
+ * * pkrel is the referenced relation; might be a partition, if recursing.
+ * * indexOid is the OID of the index (on pkrel) implementing this constraint.
+ * * parentConstr is the OID of a parent constraint; InvalidOid if this is a
+ *   top-level constraint.
+ * * numfks is the number of columns in the foreign key.
+ * * pkattnum is the attnum array of referenced attributes.
+ * * fkattnum is the attnum array of referencing attributes.
+ * * pf/pp/ffeqoperators are OID array of operators between columns.
+ * * numfkdelsetcols is the number of columns in the ON DELETE SET NULL/DEFAULT
+ *      (...) clause.
+ * * fkdelsetcols is the attnum array of the columns in the ON DELETE SET
+ *      NULL/DEFAULT clause.
+ * - with_period: true if this is a temporal FKs
  */
 static ObjectAddress
-addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel,
-					   Relation pkrel, Oid indexOid, Oid parentConstr,
-					   int numfks,
-					   int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators,
-					   Oid *ppeqoperators, Oid *ffeqoperators,
-					   int numfkdelsetcols, int16 *fkdelsetcols,
-					   bool old_check_ok,
-					   Oid parentDelTrigger, Oid parentUpdTrigger,
-					   bool with_period)
+addFkConstraint(Constraint *fkconstraint, Relation rel, Relation pkrel,
+				Oid indexOid, Oid parentConstr, int numfks, int16 *pkattnum,
+				int16 *fkattnum, Oid *pfeqoperators, Oid *ppeqoperators,
+				Oid *ffeqoperators, int numfkdelsetcols, int16 *fkdelsetcols,
+				bool with_period)
 {
 	ObjectAddress address;
 	Oid			constrOid;
@@ -10185,8 +10192,6 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel,
 	bool		conislocal;
 	int			coninhcount;
 	bool		connoinherit;
-	Oid			deleteTriggerOid,
-				updateTriggerOid;
 
 	/*
 	 * Verify relkind for each referenced partition.  At the top level, this
@@ -10285,12 +10290,63 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel,
 	/* make new constraint visible, in case we add more */
 	CommandCounterIncrement();
 
+	return address;
+}
+
+/*
+ * addFkRecurseReferenced
+ *		subroutine for ATAddForeignKeyConstraint; recurses on the referenced
+ *		side of the constraint
+ *
+ * If the referenced relation is a plain relation, create the necessary check
+ * triggers that implement the constraint.  If the referenced relation is a
+ * partitioned table, then we create a pg_constraint row referencing the parent
+ * of the referencing side for it and recurse on this routine for each
+ * partition.
+ *
+ * wqueue is the ALTER TABLE work queue; can be NULL when not running as part
+ * of an ALTER TABLE sequence.
+ * fkconstraint is the constraint being added.
+ * rel is the root referencing relation.
+ * pkrel is the referenced relation; might be a partition, if recursing.
+ * indexOid is the OID of the index (on pkrel) implementing this constraint.
+ * parentConstr is the OID of a parent constraint; InvalidOid if this is a
+ * top-level constraint.
+ * numfks is the number of columns in the foreign key
+ * pkattnum is the attnum array of referenced attributes.
+ * fkattnum is the attnum array of referencing attributes.
+ * numfkdelsetcols is the number of columns in the ON DELETE SET NULL/DEFAULT
+ *      (...) clause
+ * fkdelsetcols is the attnum array of the columns in the ON DELETE SET
+ *      NULL/DEFAULT clause
+ * pf/pp/ffeqoperators are OID array of operators between columns.
+ * old_check_ok signals that this constraint replaces an existing one that
+ * was already validated (thus this one doesn't need validation).
+ * parentDelTrigger and parentUpdTrigger, when being recursively called on
+ * a partition, are the OIDs of the parent action triggers for DELETE and
+ * UPDATE respectively.
+ * with_period: true if this is a temporal FKs
+ */
+static void
+addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel,
+					   Relation pkrel, Oid indexOid, Oid parentConstr,
+					   int numfks,
+					   int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators,
+					   Oid *ppeqoperators, Oid *ffeqoperators,
+					   int numfkdelsetcols, int16 *fkdelsetcols,
+					   bool old_check_ok,
+					   Oid parentDelTrigger, Oid parentUpdTrigger,
+					   bool with_period)
+{
+	Oid			deleteTriggerOid,
+				updateTriggerOid;
+
 	/*
 	 * Create the action triggers that enforce the constraint.
 	 */
 	createForeignKeyActionTriggers(rel, RelationGetRelid(pkrel),
 								   fkconstraint,
-								   constrOid, indexOid,
+								   parentConstr, indexOid,
 								   parentDelTrigger, parentUpdTrigger,
 								   &deleteTriggerOid, &updateTriggerOid);
 
@@ -10309,6 +10365,7 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel,
 			AttrMap    *map;
 			AttrNumber *mapped_pkattnum;
 			Oid			partIndexId;
+			ObjectAddress address;
 
 			partRel = table_open(pd->oids[i], ShareRowExclusiveLock);
 
@@ -10333,8 +10390,14 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel,
 			if (!OidIsValid(partIndexId))
 				elog(ERROR, "index for %u not found in partition %s",
 					 indexOid, RelationGetRelationName(partRel));
+
+			address = addFkConstraint(fkconstraint, rel, partRel,
+								   partIndexId, parentConstr, numfks,
+								   mapped_pkattnum, fkattnum,
+								   pfeqoperators, ppeqoperators, ffeqoperators,
+								   numfkdelsetcols, fkdelsetcols, with_period);
 			addFkRecurseReferenced(wqueue, fkconstraint, rel, partRel,
-								   partIndexId, constrOid, numfks,
+								   partIndexId, address.objectId, numfks,
 								   mapped_pkattnum, fkattnum,
 								   pfeqoperators, ppeqoperators, ffeqoperators,
 								   numfkdelsetcols, fkdelsetcols,
@@ -10351,8 +10414,6 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel,
 			}
 		}
 	}
-
-	return address;
 }
 
 /*
@@ -10718,6 +10779,7 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
 		int			numfkdelsetcols;
 		AttrNumber	confdelsetcols[INDEX_MAX_KEYS];
 		Constraint *fkconstraint;
+		ObjectAddress address;
 		Oid			deleteTriggerOid,
 					updateTriggerOid;
 
@@ -10751,7 +10813,7 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
 		 * Because we're only expanding the key space at the referenced side,
 		 * we don't need to prevent any operation in the referencing table, so
 		 * AccessShareLock suffices (assumes that dropping the constraint
-		 * acquires AEL).
+		 * acquires AccessExclusiveLock).
 		 */
 		fkRel = table_open(constrForm->conrelid, AccessShareLock);
 
@@ -10817,12 +10879,17 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
 									constrForm->confrelid, constrForm->conrelid,
 									&deleteTriggerOid, &updateTriggerOid);
 
+		address = addFkConstraint(fkconstraint, fkRel, partitionRel, partIndexId,
+								  constrOid, numfks, mapped_confkey, conkey,
+								  conpfeqop, conppeqop, conffeqop,
+								  numfkdelsetcols, confdelsetcols,
+								  constrForm->conperiod);
 		addFkRecurseReferenced(NULL,
 							   fkconstraint,
 							   fkRel,
 							   partitionRel,
 							   partIndexId,
-							   constrOid,
+							   address.objectId,
 							   numfks,
 							   mapped_confkey,
 							   conkey,
@@ -11279,6 +11346,83 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk,
 	TriggerSetParentTrigger(trigrel, updateTriggerOid, parentUpdTrigger,
 							partRelid);
 
+	/*
+	 * If the referenced table is partitioned, then the partition we're
+	 * attaching now has extra pg_constraint rows and action triggers that are
+	 * no longer needed.  Remove those.
+	 */
+	if (get_rel_relkind(fk->confrelid) == RELKIND_PARTITIONED_TABLE)
+	{
+		Relation	pg_constraint = table_open(ConstraintRelationId, RowShareLock);
+		ObjectAddresses *objs;
+		HeapTuple	consttup;
+
+		ScanKeyInit(&key,
+					Anum_pg_constraint_conrelid,
+					BTEqualStrategyNumber, F_OIDEQ,
+					ObjectIdGetDatum(fk->conrelid));
+
+		scan = systable_beginscan(pg_constraint,
+								  ConstraintRelidTypidNameIndexId,
+								  true, NULL, 1, &key);
+		objs = new_object_addresses();
+		while ((consttup = systable_getnext(scan)) != NULL)
+		{
+			Form_pg_constraint conform = (Form_pg_constraint) GETSTRUCT(consttup);
+
+			if (conform->conparentid != fk->conoid)
+				continue;
+			else
+			{
+				ObjectAddress addr;
+				int			n;
+				SysScanDesc scan2;
+				ScanKeyData key2;
+
+				ObjectAddressSet(addr, ConstraintRelationId, conform->oid);
+				add_exact_object_address(&addr, objs);
+
+				/*
+				 * First we must delete the dependency records that bind
+				 * the constraint records together.
+				 */
+				n = deleteDependencyRecordsForSpecific(ConstraintRelationId,
+													   conform->oid,
+													   DEPENDENCY_INTERNAL,
+													   ConstraintRelationId,
+													   fk->conoid);
+				if (n != 1)
+					elog(WARNING, "oops: found %d instead of 1 deps from %u to %u",
+						 n, conform->oid, fk->conoid);
+
+				/*
+				 * Now search for the triggers for this constraint and set
+				 * them up for deletion too
+				 */
+				ScanKeyInit(&key2,
+							Anum_pg_trigger_tgconstraint,
+							BTEqualStrategyNumber, F_OIDEQ,
+							ObjectIdGetDatum(conform->oid));
+				scan2 = systable_beginscan(trigrel, TriggerConstraintIndexId,
+										   true, NULL, 1, &key2);
+				while ((trigtup = systable_getnext(scan2)) != NULL)
+				{
+					ObjectAddressSet(addr, TriggerRelationId,
+									 ((Form_pg_trigger) GETSTRUCT(trigtup))->oid);
+					add_exact_object_address(&addr, objs);
+				}
+				systable_endscan(scan2);
+			}
+		}
+		/* make the dependency deletions visible */
+		CommandCounterIncrement();
+		performMultipleDeletions(objs, DROP_RESTRICT,
+								 PERFORM_DELETION_INTERNAL);
+		systable_endscan(scan);
+
+		table_close(pg_constraint, RowShareLock);
+	}
+
 	CommandCounterIncrement();
 	return true;
 }
@@ -19293,7 +19437,7 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	}
 
 	/* Do the final part of detaching */
-	DetachPartitionFinalize(rel, partRel, concurrent, defaultPartOid);
+	DetachPartitionFinalize(wqueue, rel, partRel, concurrent, defaultPartOid);
 
 	ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel));
 
@@ -19310,8 +19454,8 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
  * transaction of the concurrent algorithm fails (crash or abort).
  */
 static void
-DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
-						Oid defaultPartOid)
+DetachPartitionFinalize(List **wqueue, Relation rel, Relation partRel,
+						bool concurrent, Oid defaultPartOid)
 {
 	Relation	classRel;
 	List	   *fks;
@@ -19347,8 +19491,11 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
 	{
 		ForeignKeyCacheInfo *fk = lfirst(cell);
 		HeapTuple	contup;
+		HeapTuple	parentConTup;
 		Form_pg_constraint conform;
+		Form_pg_constraint parentConForm;
 		Constraint *fkconstraint;
+		Oid			parentConstrOid;
 		Oid			insertTriggerOid,
 					updateTriggerOid;
 
@@ -19365,7 +19512,20 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
 			continue;
 		}
 
-		/* unset conparentid and adjust conislocal, coninhcount, etc. */
+		parentConstrOid = conform->conparentid;
+
+		Assert(OidIsValid(conform->conparentid));
+		parentConTup = SearchSysCache1(CONSTROID,
+									   ObjectIdGetDatum(parentConstrOid));
+		if (!HeapTupleIsValid(parentConTup))
+			elog(ERROR, "cache lookup failed for constraint %u",
+				 conform->conparentid);
+		parentConForm = (Form_pg_constraint) GETSTRUCT(parentConTup);
+
+		/*
+		 * The constraint on this table must be marked no longer a child of
+		 * the parent's constraint, as do its check triggers.
+		 */
 		ConstraintSetParentConstraint(fk->conoid, InvalidOid, InvalidOid);
 
 		/*
@@ -19383,35 +19543,89 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
 								RelationGetRelid(partRel));
 
 		/*
-		 * Make the action triggers on the referenced relation.  When this was
-		 * a partition the action triggers pointed to the parent rel (they
-		 * still do), but now we need separate ones of our own.
+		 * If the referenced side is partitioned (which we know because our
+		 * parent's constraint points to a different relation than ours) then
+		 * we must, in addition to the above, create pg_constraint rows that
+		 * point to each partition, each with its own action triggers.
 		 */
-		fkconstraint = makeNode(Constraint);
-		fkconstraint->contype = CONSTRAINT_FOREIGN;
-		fkconstraint->conname = pstrdup(NameStr(conform->conname));
-		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;
+		if (parentConForm->conrelid != conform->conrelid)
+		{
+			int			numfks;
+			AttrNumber	conkey[INDEX_MAX_KEYS];
+			AttrMap	   *attmap;
+			AttrNumber	confkey[INDEX_MAX_KEYS];
+			Oid			conpfeqop[INDEX_MAX_KEYS];
+			Oid			conppeqop[INDEX_MAX_KEYS];
+			Oid			conffeqop[INDEX_MAX_KEYS];
+			int			numfkdelsetcols;
+			AttrNumber	confdelsetcols[INDEX_MAX_KEYS];
+			Relation	refdRel;
+
+			DeconstructFkConstraintRow(contup,
+									   &numfks,
+									   conkey,
+									   confkey,
+									   conpfeqop,
+									   conppeqop,
+									   conffeqop,
+									   &numfkdelsetcols,
+									   confdelsetcols);
+
+			/* Create a synthetic node we'll use throughout */
+			fkconstraint = makeNode(Constraint);
+			fkconstraint->contype = CONSTRAINT_FOREIGN;
+			fkconstraint->conname = pstrdup(NameStr(conform->conname));
+			fkconstraint->deferrable = conform->condeferrable;
+			fkconstraint->initdeferred = conform->condeferred;
+			fkconstraint->skip_validation = true;
+			fkconstraint->initially_valid = true;
+			/* a few irrelevant fields omitted here */
+			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->location = -1;
+
+			attmap = build_attrmap_by_name(RelationGetDescr(partRel),
+								   RelationGetDescr(rel),
+								   false);
+			for (int i = 0; i < numfks; i++)
+			{
+				Form_pg_attribute att;
 
-		createForeignKeyActionTriggers(partRel, conform->confrelid,
-									   fkconstraint, fk->conoid,
+				att = TupleDescAttr(RelationGetDescr(partRel),
+									attmap->attnums[conkey[i] - 1] - 1);
+				fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs,
+												 makeString(NameStr(att->attname)));
+			}
+
+			refdRel = table_open(fk->confrelid, AccessShareLock);
+
+			addFkRecurseReferenced(wqueue, fkconstraint, partRel,
+									   refdRel,
 									   conform->conindid,
+									   fk->conoid,
+									   numfks,
+									   confkey,
+									   conkey,
+									   conpfeqop,
+									   conppeqop,
+									   conffeqop,
+									   numfkdelsetcols,
+									   confdelsetcols,
+									   true,
 									   InvalidOid, InvalidOid,
-									   NULL, NULL);
+									   conform->conperiod);
+			table_close(refdRel, AccessShareLock);
+		}
 
 		ReleaseSysCache(contup);
+		ReleaseSysCache(parentConTup);
 	}
 	list_free_deep(fks);
 	if (trigrel)
@@ -19558,7 +19772,7 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
  * completion; this completes the detaching process.
  */
 static ObjectAddress
-ATExecDetachPartitionFinalize(Relation rel, RangeVar *name)
+ATExecDetachPartitionFinalize(List **wqueue, Relation rel, RangeVar *name)
 {
 	Relation	partRel;
 	ObjectAddress address;
@@ -19576,7 +19790,7 @@ ATExecDetachPartitionFinalize(Relation rel, RangeVar *name)
 	 */
 	WaitForOlderSnapshots(snap->xmin, false);
 
-	DetachPartitionFinalize(rel, partRel, true, InvalidOid);
+	DetachPartitionFinalize(wqueue, rel, partRel, true, InvalidOid);
 
 	ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel));
 
-- 
2.46.0

>From a5a79e9046e5b527144a0b725e1edb5a58546334 Mon Sep 17 00:00:00 2001
From: Jehan-Guillaume de Rorthais <j...@dalibo.com>
Date: Tue, 24 Sep 2024 18:10:33 +0200
Subject: [PATCH v3 3/4] Use addFkConstraint() in addFkRecurseReferencing()

---
 src/backend/commands/tablecmds.c | 124 ++++++++++++-------------------
 1 file changed, 49 insertions(+), 75 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f7e0b8fa87..9dd4f2e94f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -348,6 +348,14 @@ typedef struct ForeignTruncateInfo
 	List	   *rels;
 } ForeignTruncateInfo;
 
+typedef enum FKConstraintSide
+{
+	FK_BOTH_SIDE,
+	FK_REFERENCED_SIDE,
+	FK_REFERENCING_SIDE
+}
+FKConstraintSide;
+
 /*
  * Partition tables are expected to be dropped when the parent partitioned
  * table gets dropped. Hence for partitioning we use AUTO dependency.
@@ -509,7 +517,8 @@ static ObjectAddress ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *
 											   bool recurse, bool recursing,
 											   LOCKMODE lockmode);
 static ObjectAddress addFkConstraint(Constraint *fkconstraint, Relation rel,
-									 Relation pkrel, Oid indexOid, Oid parentConstr,
+									 Relation pkrel, Oid indexOid,
+									 Oid parentConstr, FKConstraintSide fkside,
 									 int numfks, int16 *pkattnum, int16 *fkattnum,
 									 Oid *pfeqoperators, Oid *ppeqoperators,
 									 Oid *ffeqoperators, int numfkdelsetcols,
@@ -10072,6 +10081,7 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	address = addFkConstraint(fkconstraint, rel, pkrel,
 							  indexOid,
 							  InvalidOid,	/* no parent constraint */
+							  FK_BOTH_SIDE,
 							  numfks,
 							  pkattnum,
 							  fkattnum,
@@ -10169,6 +10179,7 @@ validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums,
  * * indexOid is the OID of the index (on pkrel) implementing this constraint.
  * * parentConstr is the OID of a parent constraint; InvalidOid if this is a
  *   top-level constraint.
+ * * fkside is the side of FK related to this inherited constraint.
  * * numfks is the number of columns in the foreign key.
  * * pkattnum is the attnum array of referenced attributes.
  * * fkattnum is the attnum array of referencing attributes.
@@ -10181,8 +10192,9 @@ validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums,
  */
 static ObjectAddress
 addFkConstraint(Constraint *fkconstraint, Relation rel, Relation pkrel,
-				Oid indexOid, Oid parentConstr, int numfks, int16 *pkattnum,
-				int16 *fkattnum, Oid *pfeqoperators, Oid *ppeqoperators,
+				Oid indexOid, Oid parentConstr, FKConstraintSide fkside,
+				int numfks, int16 *pkattnum, int16 *fkattnum,
+				Oid *pfeqoperators, Oid *ppeqoperators,
 				Oid *ffeqoperators, int numfkdelsetcols, int16 *fkdelsetcols,
 				bool with_period)
 {
@@ -10273,18 +10285,31 @@ addFkConstraint(Constraint *fkconstraint, Relation rel, Relation pkrel,
 
 	ObjectAddressSet(address, ConstraintRelationId, constrOid);
 
-	/*
-	 * Mark the child constraint as part of the parent constraint; it must not
-	 * be dropped on its own.  (This constraint is deleted when the partition
-	 * is detached, but a special check needs to occur that the partition
-	 * contains no referenced values.)
-	 */
 	if (OidIsValid(parentConstr))
 	{
 		ObjectAddress referenced;
 
 		ObjectAddressSet(referenced, ConstraintRelationId, parentConstr);
-		recordDependencyOn(&address, &referenced, DEPENDENCY_INTERNAL);
+		if (fkside == FK_REFERENCED_SIDE)
+		{
+			/*
+			 * Mark the child constraint as part of the parent constraint; it must not
+			 * be dropped on its own.  (This constraint is deleted when the partition
+			 * is detached, but a special check needs to occur that the partition
+			 * contains no referenced values.)
+			 */
+			recordDependencyOn(&address, &referenced, DEPENDENCY_INTERNAL);
+		}
+		else
+		{
+			/*
+			 * Give this constraint partition-type dependencies on the parent
+			 * constraint as well as the table.
+			 */
+			recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_PRI);
+			ObjectAddressSet(referenced, RelationRelationId, RelationGetRelid(rel));
+			recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_SEC);
+		}
 	}
 
 	/* make new constraint visible, in case we add more */
@@ -10392,8 +10417,8 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel,
 					 indexOid, RelationGetRelationName(partRel));
 
 			address = addFkConstraint(fkconstraint, rel, partRel,
-								   partIndexId, parentConstr, numfks,
-								   mapped_pkattnum, fkattnum,
+								   partIndexId, parentConstr, FK_REFERENCED_SIDE,
+								   numfks, mapped_pkattnum, fkattnum,
 								   pfeqoperators, ppeqoperators, ffeqoperators,
 								   numfkdelsetcols, fkdelsetcols, with_period);
 			addFkRecurseReferenced(wqueue, fkconstraint, rel, partRel,
@@ -10540,10 +10565,7 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel,
 			AttrMap    *attmap;
 			AttrNumber	mapped_fkattnum[INDEX_MAX_KEYS];
 			bool		attached;
-			char	   *conname;
-			Oid			constrOid;
-			ObjectAddress address,
-						referenced;
+			ObjectAddress address;
 			ListCell   *cell;
 
 			CheckAlterTableIsSafe(partition);
@@ -10586,66 +10608,18 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel,
 			/*
 			 * No luck finding a good constraint to reuse; create our own.
 			 */
-			if (ConstraintNameIsUsed(CONSTRAINT_RELATION,
-									 RelationGetRelid(partition),
-									 fkconstraint->conname))
-				conname = ChooseConstraintName(RelationGetRelationName(partition),
-											   ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs),
-											   "fkey",
-											   RelationGetNamespace(partition), NIL);
-			else
-				conname = fkconstraint->conname;
-			constrOid =
-				CreateConstraintEntry(conname,
-									  RelationGetNamespace(partition),
-									  CONSTRAINT_FOREIGN,
-									  fkconstraint->deferrable,
-									  fkconstraint->initdeferred,
-									  fkconstraint->initially_valid,
-									  parentConstr,
-									  partitionId,
-									  mapped_fkattnum,
-									  numfks,
-									  numfks,
-									  InvalidOid,
-									  indexOid,
-									  RelationGetRelid(pkrel),
-									  pkattnum,
-									  pfeqoperators,
-									  ppeqoperators,
-									  ffeqoperators,
-									  numfks,
-									  fkconstraint->fk_upd_action,
-									  fkconstraint->fk_del_action,
-									  fkdelsetcols,
-									  numfkdelsetcols,
-									  fkconstraint->fk_matchtype,
-									  NULL,
-									  NULL,
-									  NULL,
-									  false,
-									  1,
-									  false,
-									  with_period,	/* conPeriod */
-									  false);
-
-			/*
-			 * Give this constraint partition-type dependencies on the parent
-			 * constraint as well as the table.
-			 */
-			ObjectAddressSet(address, ConstraintRelationId, constrOid);
-			ObjectAddressSet(referenced, ConstraintRelationId, parentConstr);
-			recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_PRI);
-			ObjectAddressSet(referenced, RelationRelationId, partitionId);
-			recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_SEC);
-
-			/* Make all this visible before recursing */
-			CommandCounterIncrement();
+			address = addFkConstraint(fkconstraint,
+									  partition, pkrel, indexOid, parentConstr,
+									  FK_REFERENCING_SIDE, numfks, pkattnum,
+									  mapped_fkattnum, pfeqoperators,
+									  ppeqoperators, ffeqoperators,
+									  numfkdelsetcols, fkdelsetcols,
+									  with_period);
 
 			/* call ourselves to finalize the creation and we're done */
 			addFkRecurseReferencing(wqueue, fkconstraint, partition, pkrel,
 									indexOid,
-									constrOid,
+									address.objectId,
 									numfks,
 									pkattnum,
 									mapped_fkattnum,
@@ -10880,9 +10854,9 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
 									&deleteTriggerOid, &updateTriggerOid);
 
 		address = addFkConstraint(fkconstraint, fkRel, partitionRel, partIndexId,
-								  constrOid, numfks, mapped_confkey, conkey,
-								  conpfeqop, conppeqop, conffeqop,
-								  numfkdelsetcols, confdelsetcols,
+								  constrOid, FK_REFERENCED_SIDE, numfks,
+								  mapped_confkey, conkey, conpfeqop, conppeqop,
+								  conffeqop, numfkdelsetcols, confdelsetcols,
 								  constrForm->conperiod);
 		addFkRecurseReferenced(NULL,
 							   fkconstraint,
-- 
2.46.0

>From abea3cc9c2236d7c2b2d973317dcde31b6553e31 Mon Sep 17 00:00:00 2001
From: Jehan-Guillaume de Rorthais <j...@dalibo.com>
Date: Wed, 25 Sep 2024 13:31:07 +0200
Subject: [PATCH v3 4/4] Use addFkConstraint() in CloneFkReferencing()

---
 src/backend/commands/tablecmds.c | 124 +++++++++++--------------------
 1 file changed, 43 insertions(+), 81 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 9dd4f2e94f..6a1d06ecd5 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -516,13 +516,15 @@ static ObjectAddress ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *
 											   Relation rel, Constraint *fkconstraint,
 											   bool recurse, bool recursing,
 											   LOCKMODE lockmode);
-static ObjectAddress addFkConstraint(Constraint *fkconstraint, Relation rel,
+static ObjectAddress addFkConstraint(char *constraintname,
+									 Constraint *fkconstraint, Relation rel,
 									 Relation pkrel, Oid indexOid,
 									 Oid parentConstr, FKConstraintSide fkside,
 									 int numfks, int16 *pkattnum, int16 *fkattnum,
 									 Oid *pfeqoperators, Oid *ppeqoperators,
 									 Oid *ffeqoperators, int numfkdelsetcols,
-									 int16 *fkdelsetcols, bool with_period);
+									 int16 *fkdelsetcols, bool is_internal,
+									 bool with_period);
 static void addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint,
 											Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr,
 											int numfks, int16 *pkattnum, int16 *fkattnum,
@@ -10078,7 +10080,7 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	 * Create all the constraint and trigger objects, recursing to partitions
 	 * as necessary.  First handle the referenced side.
 	 */
-	address = addFkConstraint(fkconstraint, rel, pkrel,
+	address = addFkConstraint(fkconstraint->conname, fkconstraint, rel, pkrel,
 							  indexOid,
 							  InvalidOid,	/* no parent constraint */
 							  FK_BOTH_SIDE,
@@ -10090,6 +10092,7 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 							  ffeqoperators,
 							  numfkdelsetcols,
 							  fkdelsetcols,
+							  false,
 							  with_period);
 
 	addFkRecurseReferenced(wqueue, fkconstraint, rel, pkrel,
@@ -10173,6 +10176,8 @@ validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums,
  *
  * Creates a pg_constraint rows referencing a parent if given.
  *
+ * * constraintname the base name for the constraint being added. This is
+ *   copied to fkconstraint->conname if the later is not set.
  * * fkconstraint is the constraint being added.
  * * rel is the root referencing relation.
  * * pkrel is the referenced relation; might be a partition, if recursing.
@@ -10191,12 +10196,12 @@ validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums,
  * - with_period: true if this is a temporal FKs
  */
 static ObjectAddress
-addFkConstraint(Constraint *fkconstraint, Relation rel, Relation pkrel,
-				Oid indexOid, Oid parentConstr, FKConstraintSide fkside,
-				int numfks, int16 *pkattnum, int16 *fkattnum,
-				Oid *pfeqoperators, Oid *ppeqoperators,
+addFkConstraint(char *constraintname, Constraint *fkconstraint, Relation rel,
+				Relation pkrel, Oid indexOid, Oid parentConstr,
+				FKConstraintSide fkside, int numfks, int16 *pkattnum,
+				int16 *fkattnum, Oid *pfeqoperators, Oid *ppeqoperators,
 				Oid *ffeqoperators, int numfkdelsetcols, int16 *fkdelsetcols,
-				bool with_period)
+				bool is_internal, bool with_period)
 {
 	ObjectAddress address;
 	Oid			constrOid;
@@ -10222,13 +10227,16 @@ addFkConstraint(Constraint *fkconstraint, Relation rel, Relation pkrel,
 	 */
 	if (ConstraintNameIsUsed(CONSTRAINT_RELATION,
 							 RelationGetRelid(rel),
-							 fkconstraint->conname))
+							 constraintname))
 		conname = ChooseConstraintName(RelationGetRelationName(rel),
 									   ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs),
 									   "fkey",
 									   RelationGetNamespace(rel), NIL);
 	else
-		conname = fkconstraint->conname;
+		conname = constraintname;
+
+	if (fkconstraint->conname == NULL)
+		fkconstraint->conname = pstrdup(conname);
 
 	if (OidIsValid(parentConstr))
 	{
@@ -10281,7 +10289,7 @@ addFkConstraint(Constraint *fkconstraint, Relation rel, Relation pkrel,
 									  coninhcount,	/* inhcount */
 									  connoinherit, /* conNoInherit */
 									  with_period,	/* conPeriod */
-									  false);	/* is_internal */
+									  is_internal);	/* is_internal */
 
 	ObjectAddressSet(address, ConstraintRelationId, constrOid);
 
@@ -10416,11 +10424,12 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel,
 				elog(ERROR, "index for %u not found in partition %s",
 					 indexOid, RelationGetRelationName(partRel));
 
-			address = addFkConstraint(fkconstraint, rel, partRel,
-								   partIndexId, parentConstr, FK_REFERENCED_SIDE,
-								   numfks, mapped_pkattnum, fkattnum,
-								   pfeqoperators, ppeqoperators, ffeqoperators,
-								   numfkdelsetcols, fkdelsetcols, with_period);
+			address = addFkConstraint(fkconstraint->conname, fkconstraint, rel,
+								   partRel, partIndexId, parentConstr,
+								   FK_REFERENCED_SIDE, numfks, mapped_pkattnum,
+								   fkattnum, pfeqoperators, ppeqoperators,
+								   ffeqoperators, numfkdelsetcols,
+								   fkdelsetcols, false, with_period);
 			addFkRecurseReferenced(wqueue, fkconstraint, rel, partRel,
 								   partIndexId, address.objectId, numfks,
 								   mapped_pkattnum, fkattnum,
@@ -10608,12 +10617,12 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel,
 			/*
 			 * No luck finding a good constraint to reuse; create our own.
 			 */
-			address = addFkConstraint(fkconstraint,
+			address = addFkConstraint(fkconstraint->conname, fkconstraint,
 									  partition, pkrel, indexOid, parentConstr,
 									  FK_REFERENCING_SIDE, numfks, pkattnum,
 									  mapped_fkattnum, pfeqoperators,
 									  ppeqoperators, ffeqoperators,
-									  numfkdelsetcols, fkdelsetcols,
+									  numfkdelsetcols, fkdelsetcols, false,
 									  with_period);
 
 			/* call ourselves to finalize the creation and we're done */
@@ -10853,10 +10862,11 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
 									constrForm->confrelid, constrForm->conrelid,
 									&deleteTriggerOid, &updateTriggerOid);
 
-		address = addFkConstraint(fkconstraint, fkRel, partitionRel, partIndexId,
-								  constrOid, FK_REFERENCED_SIDE, numfks,
-								  mapped_confkey, conkey, conpfeqop, conppeqop,
-								  conffeqop, numfkdelsetcols, confdelsetcols,
+		address = addFkConstraint(fkconstraint->conname, fkconstraint, fkRel,
+								  partitionRel, partIndexId, constrOid,
+								  FK_REFERENCED_SIDE, numfks, mapped_confkey,
+								  conkey, conpfeqop, conppeqop, conffeqop,
+								  numfkdelsetcols, confdelsetcols, false,
 								  constrForm->conperiod);
 		addFkRecurseReferenced(NULL,
 							   fkconstraint,
@@ -10979,9 +10989,7 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel)
 		Constraint *fkconstraint;
 		bool		attached;
 		Oid			indexOid;
-		Oid			constrOid;
-		ObjectAddress address,
-					referenced;
+		ObjectAddress address;
 		ListCell   *lc;
 		Oid			insertTriggerOid,
 					updateTriggerOid;
@@ -11079,7 +11087,7 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel)
 		fkconstraint->old_conpfeqop = NIL;
 		fkconstraint->old_pktable_oid = InvalidOid;
 		fkconstraint->skip_validation = false;
-		fkconstraint->initially_valid = true;
+		fkconstraint->initially_valid = constrForm->convalidated;
 		for (int i = 0; i < numfks; i++)
 		{
 			Form_pg_attribute att;
@@ -11089,73 +11097,27 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel)
 			fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs,
 											 makeString(NameStr(att->attname)));
 		}
-		if (ConstraintNameIsUsed(CONSTRAINT_RELATION,
-								 RelationGetRelid(partRel),
-								 NameStr(constrForm->conname)))
-			fkconstraint->conname =
-				ChooseConstraintName(RelationGetRelationName(partRel),
-									 ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs),
-									 "fkey",
-									 RelationGetNamespace(partRel), NIL);
-		else
-			fkconstraint->conname = pstrdup(NameStr(constrForm->conname));
 
 		indexOid = constrForm->conindid;
 		with_period = constrForm->conperiod;
-		constrOid =
-			CreateConstraintEntry(fkconstraint->conname,
-								  constrForm->connamespace,
-								  CONSTRAINT_FOREIGN,
-								  fkconstraint->deferrable,
-								  fkconstraint->initdeferred,
-								  constrForm->convalidated,
-								  parentConstrOid,
-								  RelationGetRelid(partRel),
-								  mapped_conkey,
-								  numfks,
-								  numfks,
-								  InvalidOid,	/* not a domain constraint */
-								  indexOid,
-								  constrForm->confrelid,	/* same foreign rel */
-								  confkey,
-								  conpfeqop,
-								  conppeqop,
-								  conffeqop,
-								  numfks,
-								  fkconstraint->fk_upd_action,
-								  fkconstraint->fk_del_action,
-								  confdelsetcols,
-								  numfkdelsetcols,
-								  fkconstraint->fk_matchtype,
-								  NULL,
-								  NULL,
-								  NULL,
-								  false,	/* islocal */
-								  1,	/* inhcount */
-								  false,	/* conNoInherit */
-								  with_period,	/* conPeriod */
-								  true);
-
-		/* Set up partition dependencies for the new constraint */
-		ObjectAddressSet(address, ConstraintRelationId, constrOid);
-		ObjectAddressSet(referenced, ConstraintRelationId, parentConstrOid);
-		recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_PRI);
-		ObjectAddressSet(referenced, RelationRelationId,
-						 RelationGetRelid(partRel));
-		recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_SEC);
+
+		address = addFkConstraint(NameStr(constrForm->conname), fkconstraint,
+								  partRel, pkrel, indexOid, parentConstrOid,
+								  FK_REFERENCING_SIDE, numfks, confkey,
+								  mapped_conkey, conpfeqop,
+								  conppeqop, conffeqop,
+								  numfkdelsetcols, confdelsetcols,
+								  true, with_period);
 
 		/* Done with the cloned constraint's tuple */
 		ReleaseSysCache(tuple);
 
-		/* Make all this visible before recursing */
-		CommandCounterIncrement();
-
 		addFkRecurseReferencing(wqueue,
 								fkconstraint,
 								partRel,
 								pkrel,
 								indexOid,
-								constrOid,
+								address.objectId,
 								numfks,
 								confkey,
 								mapped_conkey,
-- 
2.46.0

Reply via email to