From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Tender Wang <tndrwang@gmail.com>
Cc: Junwang Zhao <zhjwpku@gmail.com>
Cc: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Cc: Alexander Lakhin <exclusion@gmail.com>
Cc: Baehler Thomas SBB CFF FFS <thomas.baehler2@sbb.ch>
Cc: Guillaume Lelarge <guillaume@lelarge.info>
Subject: Re: [BUG] Fix DETACH with FK pointing to a partitioned table fails
Date: Thu, 22 Aug 2024 14:41:50 -0400
Message-ID: <202408221841.hecaenywo5nm@alvherre.pgsql> (raw)
In-Reply-To: <CAHewXNmf6sCai+iYBSrTW8D8NWBaX+zNYV25=iHWM17zbEfF8g@mail.gmail.com>
On 2024-Aug-22, Tender Wang wrote:
> I apply the v14 patch on branch REL_14_STABLE. I run this thread issue and I
> find below error.
> [...]
> ERROR: cache lookup failed for constraint 16400
>
> I haven't look into details to find out where cause above error.
Right, we try to drop the constraint twice. We can dodge this by
collecting all constraints to drop in the loop and process them in a
single performMultipleDeletions, as in the attached v14-2.
TBH I think it's a bit infuriating that we lose the constraint (which
was explicitly declared) because of ATTACH/DETACH. So the behavior of
v15 and above is better.
> By the way, I run above SQL sequences on REL_14_STABLE without your
> partch. I didn't find reporting error, and running oidjoins.sql
> didn't report warnings. Do I miss something?
I think the action triggers are missing, so if you keep rows in the r_1
table after you've detached them, you can still delete them from the
referenced table p, instead of getting the error that you should get.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"In fact, the basic problem with Perl 5's subroutines is that they're not
crufty enough, so the cruft leaks out into user-defined code instead, by
the Conservation of Cruft Principle." (Larry Wall, Apocalypse 6)
Attachments:
[text/x-diff] v14-2-0001-drop-constraint-when-detaching.patch (5.1K, ../202408221841.hecaenywo5nm@alvherre.pgsql/2-v14-2-0001-drop-constraint-when-detaching.patch)
download | inline diff:
From 43fc7215c1519e698fe1d58edd46f87da8529186 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@alvh.no-ip.org>
Date: Wed, 21 Aug 2024 17:47:03 -0400
Subject: [PATCH v14-2] drop constraint when detaching
---
src/backend/commands/tablecmds.c | 84 ++++++++++++++++++++++----------
1 file changed, 58 insertions(+), 26 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex c39e6799ca..3a8ffe30eb 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -18129,6 +18129,7 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
new_repl[Natts_pg_class];
HeapTuple tuple,
newtuple;
+ ObjectAddresses *dropobjs = NULL;
if (concurrent)
{
@@ -18143,8 +18144,8 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
DropClonedTriggersFromPartition(RelationGetRelid(partRel));
/*
- * Detach any foreign keys that are inherited. This includes creating- * additional action triggers.+ * Detach any foreign keys that are inherited -- or, if they reference+ * partitioned tables, drop them.
*/
fks = copyObject(RelationGetFKeyList(partRel));
foreach(cell, fks)
@@ -18152,7 +18153,6 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
ForeignKeyCacheInfo *fk = lfirst(cell);
HeapTuple contup;
Form_pg_constraint conform;
- Constraint *fkconstraint;
contup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(fk->conoid));
if (!HeapTupleIsValid(contup))
@@ -18167,39 +18167,71 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
continue;
}
- /* unset conparentid and adjust conislocal, coninhcount, etc. */+ /* Mark the constraint as independent */
ConstraintSetParentConstraint(fk->conoid, InvalidOid, InvalidOid);
/*
- * 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 constraint references a partitioned table, just drop the+ * constraint, because it's more work to preserve the constraint+ * correctly.+ *+ * If it references a plain table, then we can create the action+ * triggers and it'll be okay.
*/
- 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->old_conpfeqop = NIL;- fkconstraint->old_pktable_oid = InvalidOid;- fkconstraint->skip_validation = false;- fkconstraint->initially_valid = true;+ if (get_rel_relkind(fk->confrelid) == RELKIND_PARTITIONED_TABLE)+ {+ ObjectAddress constraddr;- createForeignKeyActionTriggers(partRel, conform->confrelid,- fkconstraint, fk->conoid,- conform->conindid);+ /* make the dependency deletions above visible */+ CommandCounterIncrement();++ /*+ * Remember the constraint and its triggers for later deletion.+ */+ if (dropobjs == NULL)+ dropobjs = new_object_addresses();+ ObjectAddressSet(constraddr, ConstraintRelationId, fk->conoid);+ add_exact_object_address(&constraddr, dropobjs);+ }+ else+ {+ Constraint *fkconstraint;++ /*+ * 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.+ */+ 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->old_conpfeqop = NIL;+ fkconstraint->old_pktable_oid = InvalidOid;+ fkconstraint->skip_validation = false;+ fkconstraint->initially_valid = true;++ createForeignKeyActionTriggers(partRel, conform->confrelid,+ fkconstraint, fk->conoid,+ conform->conindid);+ }
ReleaseSysCache(contup);
}
list_free_deep(fks);
+ /* If we collected any constraints for deletion, do so now. */+ if (dropobjs != NULL)+ performMultipleDeletions(dropobjs, DROP_CASCADE, 0);+
/*
* Any sub-constraints that are in the referenced-side of a larger
* constraint have to be removed. This partition is no longer part of the
--
2.39.2
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: pgsql-hackers@postgresql.org
Cc: alvherre@alvh.no-ip.org, tndrwang@gmail.com, zhjwpku@gmail.com, jgdr@dalibo.com, exclusion@gmail.com, thomas.baehler2@sbb.ch, guillaume@lelarge.info
Subject: Re: [BUG] Fix DETACH with FK pointing to a partitioned table fails
In-Reply-To: <202408221841.hecaenywo5nm@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox