Hi,

(patch proposal below).

Consider a table with a FK pointing to a partitioned table.

  CREATE TABLE p ( id bigint PRIMARY KEY )
    PARTITION BY list (id);
  CREATE TABLE p_1 PARTITION OF p FOR VALUES IN (1);

  CREATE TABLE r_1 (
    id   bigint PRIMARY KEY,
    p_id bigint NOT NULL,
    FOREIGN KEY (p_id) REFERENCES p (id)
  );

Now, attach this table "refg_1" as partition of another one having the same FK:

  CREATE TABLE r (
    id   bigint PRIMARY KEY,
    p_id bigint NOT NULL,
    FOREIGN KEY (p_id) REFERENCES p (id)
  ) PARTITION BY list (id);

  ALTER TABLE r ATTACH PARTITION r_1 FOR VALUES IN (1); 

The old sub-FKs (below 18289) created in this table to enforce the action
triggers on referenced partitions are not deleted when the table becomes a
partition. Because of this, we have additional and useless triggers on the
referenced partitions and we can not DETACH this partition on the referencing
side anymore:

  => ALTER TABLE r DETACH PARTITION r_1;
  ERROR:  could not find ON INSERT check triggers of foreign key
          constraint 18289

  => SELECT c.oid, conparentid, 
       conrelid::regclass, 
       confrelid::regclass, 
       t.tgfoid::regproc
     FROM pg_constraint c 
     JOIN pg_trigger t ON t.tgconstraint = c.oid
     WHERE confrelid::regclass = 'p_1'::regclass;
    oid  │ conparentid │ conrelid │ confrelid │         tgfoid         
  ───────┼─────────────┼──────────┼───────────┼────────────────────────
   18289 │       18286 │ r_1      │ p_1       │ "RI_FKey_noaction_del"
   18289 │       18286 │ r_1      │ p_1       │ "RI_FKey_noaction_upd"
   18302 │       18299 │ r        │ p_1       │ "RI_FKey_noaction_del"
   18302 │       18299 │ r        │ p_1       │ "RI_FKey_noaction_upd"
  (4 rows)

The legitimate constraint and triggers here are 18302. The old sub-FK
18289 having 18286 as parent should have gone during the ATTACH PARTITION.

Please, find in attachment a patch dropping old "sub-FK" during the ATTACH
PARTITION command and adding a regression test about it. At the very least, it
help understanding the problem and sketch a possible solution.

Regards,
>From 5fc7997b9f9a17ee5a31f059c18e6c01fd716c04 Mon Sep 17 00:00:00 2001
From: Jehan-Guillaume de Rorthais <j...@dalibo.com>
Date: Wed, 5 Jul 2023 19:19:40 +0200
Subject: [PATCH v1] Remove useless parted-FK constraints when attaching a
 partition

When a FK is referencing a partitioned table, this FK is parenting
as many other "parted-FK" constraints than the referencing side has
partitions. Each of these sub-constraints enforce only the
UPDATE/DELETE triggers on each referenced partition, but not the
check triggers on the referencing partition as this is already
handle by the parent constraint. These parted-FK are half-backed on
purpose.

However when attaching such standalone table to a partitioned table
having the same FK definition, all these sub-constraints were not
removed. This leave a useless action trigger on each referenced
partition, the legitimate one already checking against the root of
the referencing partition.

Moreover, when the partition is later detached,
DetachPartitionFinalize() look for all existing FK on the relation
and try to detach them from the parent triggers and constraints.
But because these parted-FK are half backed, calling
GetForeignKeyCheckTriggers() to later detach the check triggers
raise an ERROR:

  ERROR:  could not find ON INSERT check triggers of foreign key
          constraint NNNNN.
---
 src/backend/commands/tablecmds.c          | 57 +++++++++++++++++++++--
 src/test/regress/expected/foreign_key.out | 22 +++++++++
 src/test/regress/sql/foreign_key.sql      | 27 +++++++++++
 3 files changed, 103 insertions(+), 3 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index fce5e6f220..7c1aa5d395 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -10566,9 +10566,11 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk,
 	Form_pg_constraint parentConstr;
 	HeapTuple	partcontup;
 	Form_pg_constraint partConstr;
-	ScanKeyData key;
+	ScanKeyData key[3];
 	SysScanDesc scan;
 	HeapTuple	trigtup;
+	HeapTuple	contup;
+	Relation	pg_constraint;
 	Oid			insertTriggerOid,
 				updateTriggerOid;
 
@@ -10631,12 +10633,12 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk,
 	 * in the partition.  We identify them because they have our constraint
 	 * OID, as well as being on the referenced rel.
 	 */
-	ScanKeyInit(&key,
+	ScanKeyInit(key,
 				Anum_pg_trigger_tgconstraint,
 				BTEqualStrategyNumber, F_OIDEQ,
 				ObjectIdGetDatum(fk->conoid));
 	scan = systable_beginscan(trigrel, TriggerConstraintIndexId, true,
-							  NULL, 1, &key);
+							  NULL, 1, key);
 	while ((trigtup = systable_getnext(scan)) != NULL)
 	{
 		Form_pg_trigger trgform = (Form_pg_trigger) GETSTRUCT(trigtup);
@@ -10684,6 +10686,55 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk,
 	TriggerSetParentTrigger(trigrel, updateTriggerOid, parentUpdTrigger,
 							partRelid);
 
+	/*
+	 * If the referenced side of the FK is partionned, we need to remove the
+	 * action triggers from these partitions as well as a legitimate one
+	 * already exists and points to the root of the referencing partition.
+	 * These action triggers to remove are binded to a FK constraints having
+	 * this constraint as parent. Dropping these constraints cleans up
+	 * everything.
+	 */
+	pg_constraint = table_open(ConstraintRelationId, RowShareLock);
+	ScanKeyInit(&key[0],
+				Anum_pg_constraint_conrelid, BTEqualStrategyNumber,
+				F_OIDEQ, ObjectIdGetDatum(fk->conrelid));
+	ScanKeyInit(&key[1],
+				Anum_pg_constraint_conparentid, BTEqualStrategyNumber,
+				F_CHAREQ, ObjectIdGetDatum(fk->conoid));
+	ScanKeyInit(&key[2],
+				Anum_pg_constraint_contype, BTEqualStrategyNumber,
+				F_CHAREQ, CharGetDatum(CONSTRAINT_FOREIGN));
+	scan = systable_beginscan(pg_constraint, InvalidOid, true,
+							  NULL, 2, key);
+
+	while ((contup = systable_getnext(scan)) != NULL)
+	{
+		ObjectAddress conobj;
+		Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(contup);
+
+		/*
+		 * we need to remove the dependency between this old half-backend
+		 * foreign key constraint and the main one so we can later drop
+		 * it.
+		 */
+		deleteDependencyRecordsFor(ConstraintRelationId,
+								   con->oid,
+								   false);
+
+		/* make dependency deletion visible to performDeletion */
+		CommandCounterIncrement();
+
+		conobj.classId = ConstraintRelationId;
+		conobj.objectId = con->oid;
+		conobj.objectSubId = 0;
+
+		/* drop the old constraint */
+		performDeletion(&conobj, DROP_CASCADE, 0);
+	}
+	systable_endscan(scan);
+	table_close(pg_constraint, RowShareLock);
+
+
 	CommandCounterIncrement();
 	return true;
 }
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 55f7158c1a..1595739cf7 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -2917,3 +2917,25 @@ 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()
+-- Test partitioned tables on both ends of the referential constraint
+-- test attaching and detaching a partition with an existing
+-- foreign key constraint
+BEGIN;
+CREATE TABLE parted_refd (
+    id bigint,
+    PRIMARY KEY (id)
+)
+PARTITION BY list (id);
+CREATE TABLE parted_refd_1 PARTITION OF parted_refd FOR VALUES IN (1);
+CREATE TABLE parted_refg (
+  id   bigint,
+  p_id bigint NOT NULL,
+  PRIMARY KEY (id),
+  FOREIGN KEY (p_id) REFERENCES parted_refd (id)
+)
+PARTITION BY list (id);
+CREATE TABLE parted_refg_1 ( LIKE parted_refg INCLUDING ALL );
+ALTER TABLE parted_refg_1 ADD FOREIGN KEY (p_id) REFERENCES parted_refd (id);
+ALTER TABLE parted_refg ATTACH PARTITION parted_refg_1 FOR VALUES IN (1);
+ALTER TABLE parted_refg DETACH PARTITION parted_refg_1;
+ROLLBACK;
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 22e177f89b..02828d3e94 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -2069,3 +2069,30 @@ UPDATE fkpart11.pk SET a = 3 WHERE a = 4;
 UPDATE fkpart11.pk SET a = 1 WHERE a = 2;
 
 DROP SCHEMA fkpart11 CASCADE;
+
+-- Test partitioned tables on both ends of the referential constraint
+
+-- test attaching and detaching a partition with an existing
+-- foreign key constraint
+BEGIN;
+CREATE TABLE parted_refd (
+    id bigint,
+    PRIMARY KEY (id)
+)
+PARTITION BY list (id);
+CREATE TABLE parted_refd_1 PARTITION OF parted_refd FOR VALUES IN (1);
+
+CREATE TABLE parted_refg (
+  id   bigint,
+  p_id bigint NOT NULL,
+  PRIMARY KEY (id),
+  FOREIGN KEY (p_id) REFERENCES parted_refd (id)
+)
+PARTITION BY list (id);
+
+CREATE TABLE parted_refg_1 ( LIKE parted_refg INCLUDING ALL );
+ALTER TABLE parted_refg_1 ADD FOREIGN KEY (p_id) REFERENCES parted_refd (id);
+
+ALTER TABLE parted_refg ATTACH PARTITION parted_refg_1 FOR VALUES IN (1);
+ALTER TABLE parted_refg DETACH PARTITION parted_refg_1;
+ROLLBACK;
-- 
2.40.1

Reply via email to