Feedback-ID: ia2694551:Fastmail
Date: Fri, 19 Jul 2024 15:18:28 +0200
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Tender Wang <tndrwang@gmail.com>
Cc: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
 Alexander Lakhin <exclusion@gmail.com>
Subject: Re: [BUG] Fix DETACH with FK pointing to a partitioned table fails
Message-ID: <202407191318.uecnw7oaidhu@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="pbb5725efu3pjse7"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAHewXNmF-dn+hMkcsWM=b_f7Zc4nwP7EN0uXK0NG_VNEk-q1uA@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202407191318.uecnw7oaidhu%40alvherre.pgsql>
Precedence: bulk


--pbb5725efu3pjse7
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

Hello,

I think the fix for the check triggers should be as the attached.  Very
close to what you did, but you were skipping some operations that needed
to be kept.  AFAICS this patch works correctly for the posted cases.

I haven't looked at the action triggers yet; I think we need to create
one trigger for each partition of the referenced side, so we need to
loop instead of doing a single one.


I find this pair of queries useful; they show which constraints exist
and which triggers belong to each.  We need to make the constraints and
triggers match after a detach right as things would be if the
just-detached partition were an individual table having the same foreign
key.


-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"People get annoyed when you try to debug them."  (Larry Wall)

--pbb5725efu3pjse7
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename="0001-Fix-partition-detach-on-tables-with-FKs-to-partition.patch"

From 925739ef0c6b64c73b021ed930f65f140004ceb5 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Fri, 19 Jul 2024 15:12:38 +0200
Subject: [PATCH] Fix partition detach on tables with FKs to partitioned tables

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

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 721d24783b..ca777d3c88 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -19185,8 +19185,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;
 
@@ -19203,22 +19206,46 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
 			continue;
 		}
 
+		parentConstrOid = conform->conparentid;
+
 		/* unset conparentid and adjust conislocal, coninhcount, etc. */
 		ConstraintSetParentConstraint(fk->conoid, InvalidOid, InvalidOid);
 
 		/*
-		 * Also, look up the partition's "check" triggers corresponding to the
-		 * constraint being detached and detach them from the parent triggers.
+		 * Search for the partition's check triggers that implement the
+		 * constraint being detached, and make them no longer children of the
+		 * triggers on the parent table.  However, if the referenced side is a
+		 * partitioned table, there are no such check triggers (we know that
+		 * the referenced side is partitioned because our constraint row points
+		 * to a partition, whereas our parent constraint points to its parent
+		 * partitioned table.)
 		 */
-		GetForeignKeyCheckTriggers(trigrel,
-								   fk->conoid, fk->confrelid, fk->conrelid,
-								   &insertTriggerOid, &updateTriggerOid);
-		Assert(OidIsValid(insertTriggerOid));
-		TriggerSetParentTrigger(trigrel, insertTriggerOid, InvalidOid,
-								RelationGetRelid(partRel));
-		Assert(OidIsValid(updateTriggerOid));
-		TriggerSetParentTrigger(trigrel, updateTriggerOid, InvalidOid,
-								RelationGetRelid(partRel));
+		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);
+
+		if (parentConForm->conrelid != conform->conrelid)
+		{
+			/*
+			 * Also, look up the partition's "check" triggers corresponding to the
+			 * constraint being detached and detach them from the parent triggers.
+			 */
+			GetForeignKeyCheckTriggers(trigrel,
+									   fk->conoid, fk->confrelid, fk->conrelid,
+									   &insertTriggerOid, &updateTriggerOid);
+			Assert(OidIsValid(insertTriggerOid));
+			TriggerSetParentTrigger(trigrel, insertTriggerOid, InvalidOid,
+									RelationGetRelid(partRel));
+			Assert(OidIsValid(updateTriggerOid));
+			TriggerSetParentTrigger(trigrel, updateTriggerOid, InvalidOid,
+									RelationGetRelid(partRel));
+		}
+
+		ReleaseSysCache(parentConTup);
 
 		/*
 		 * Make the action triggers on the referenced relation.  When this was
-- 
2.39.2


--pbb5725efu3pjse7--