public inbox for [email protected]  
help / color / mirror / Atom feed
From: Alvaro Herrera <[email protected]>
To: Tender Wang <[email protected]>
Cc: Jehan-Guillaume de Rorthais <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Alexander Lakhin <[email protected]>
Subject: Re: [BUG] Fix DETACH with FK pointing to a partitioned table fails
Date: Fri, 19 Jul 2024 15:18:28 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAHewXNmF-dn+hMkcsWM=b_f7Zc4nwP7EN0uXK0NG_VNEk-q1uA@mail.gmail.com>

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)


Attachments:

  [text/x-diff] 0001-Fix-partition-detach-on-tables-with-FKs-to-partition.patch (3.3K, ../[email protected]/2-0001-Fix-partition-detach-on-tables-with-FKs-to-partition.patch)
  download | inline diff:
From 925739ef0c6b64c73b021ed930f65f140004ceb5 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <[email protected]>
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



view thread (7+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [BUG] Fix DETACH with FK pointing to a partitioned table fails
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox