public inbox for [email protected]
help / color / mirror / Atom feedFrom: Sami Imseih <[email protected]>
To: Amul Sul <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Bug in detaching a partition with a foreign key.
Date: Fri, 17 Jan 2025 14:44:01 -0600
Message-ID: <CAA5RZ0vk4SJ9PiD2RyG-CKOYvOFewz6QweKcp2_EegBKns=dOA@mail.gmail.com> (raw)
In-Reply-To: <CAAJ_b97GuPh6wQPbxQS-Zpy16Oh+0aMv-w64QcGrLhCOZZ6p+g@mail.gmail.com>
References: <CAAJ_b97GuPh6wQPbxQS-Zpy16Oh+0aMv-w64QcGrLhCOZZ6p+g@mail.gmail.com>
This is a bug indeed. I tried your patch, but it ends up in a seg fault.
I also see this was raised in another thread [0].
It can be reproduced in a slightly simplified case, using only a
single level partition.
"""
CREATE TABLE bar(id int PRIMARY KEY) PARTITION BY RANGE(id);
CREATE TABLE bar_p0 PARTITION OF bar FOR VALUES FROM (0) TO (100);
CREATE TABLE foo(id int) PARTITION BY RANGE(id);
CREATE TABLE foo_p0 PARTITION OF foo FOR VALUES FROM (0) TO (100);
ALTER TABLE foo_p0 ADD CONSTRAINT child_fk_con FOREIGN KEY (id) REFERENCES bar;
ALTER TABLE foo DETACH PARTITION foo_p0 ;
"""
Here is what I I found.
In DetachPartitionFinalize, after the child is detached from
the parent, the FK's insert and update triggers are then
removed from pg_depend with TriggerSetParentTrigger
/*
* 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);
/*
* 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));
Specifically, the dependency types DEPENDENCY_PARTITION_PRI and
DEPENDENCY_PARTITION_SEC are removed from pg_depend.
deleteDependencyRecordsForClass(TriggerRelationId, childTrigId,
TriggerRelationId,
DEPENDENCY_PARTITION_PRI);
deleteDependencyRecordsForClass(TriggerRelationId, childTrigId,
RelationRelationId,
DEPENDENCY_PARTITION_SEC);
In the repro case, an FK on a partition with a reference
to a partition parent table does not create a new dependency.
postgres=# SELECT count(*) total, deptype FROM pg_depend WHERE deptype
in ('P', 'S') group by deptype;
total | deptype
-------+---------
2 | P
2 | S
(2 rows)
postgres=# ALTER TABLE foo_p0 ADD CONSTRAINT child_fk_con FOREIGN KEY
(id) REFERENCES bar;
ALTER TABLE
postgres=#
postgres=# SELECT count(*) total, deptype FROM pg_depend WHERE deptype
in ('P', 'S') group by deptype;
total | deptype
-------+---------
2 | P
2 | S
(2 rows)
We also see that the FK riggers created are associated with the parent
constraint rather than the child constraint, i.e. tgconstraint = 17387
postgres=# ALTER TABLE foo_p0 ADD CONSTRAINT child_fk_con FOREIGN KEY
(id) REFERENCES bar;
ALTER TABLE
postgres=#
postgres=# SELECT oid, tgrelid::regclass relname, tgparentid, 'insert'
trigger_type, tgconstraint FROM pg_trigger WHERE tgfoid in (1644,
1645)
postgres-# and tgtype & (1 << 2) > 0
postgres-# union all
postgres-# SELECT oid, tgrelid::regclass relname, tgparentid, 'update'
trigger_type, tgconstraint FROM pg_trigger WHERE tgfoid in (1644,
1645)
postgres-# and tgtype & (1 << 4) > 0;
oid | relname | tgparentid | trigger_type | tgconstraint
-------+---------+------------+--------------+--------------
17393 | foo_p0 | 0 | insert | 17387
17394 | foo_p0 | 0 | update | 17387
(2 rows)
postgres=# ALTER TABLE foo DETACH PARTITION foo_p0 ;
ERROR: could not find ON INSERT check triggers of foreign key constraint 17390
postgres=#
postgres=# select oid, conparentid, conrelid::regclass,
confrelid::regclass from pg_constraint where oid = 17390;
oid | conparentid | conrelid | confrelid
-------+-------------+----------+-----------
17390 | 17387 | foo_p0 | bar_p0
(1 row)
postgres=# select oid, conparentid, conrelid::regclass,
confrelid::regclass from pg_constraint where oid = 17387;
oid | conparentid | conrelid | confrelid
-------+-------------+----------+-----------
17387 | 0 | foo_p0 | bar
(1 row)
This is not the case when the constraint is created on
the parent table,
i.e. ALTER TABLE foo ADD CONSTRAINT child_fk_con FOREIGN KEY (id) REFERENCES bar
In this case we also see a dependency.
postgres=# SELECT count(*) total, deptype FROM pg_depend WHERE deptype
in ('P', 'S') group by deptype;
total | deptype
-------+---------
2 | P
2 | S
(2 rows)
postgres=# ALTER TABLE foo ADD CONSTRAINT child_fk_con FOREIGN KEY
(id) REFERENCES bar;
ALTER TABLE
postgres=# SELECT count(*) total, deptype FROM pg_depend WHERE deptype
in ('P', 'S') group by deptype;
total | deptype
-------+---------
3 | P
3 | S
(2 rows)
Also, the constraint relname in the parent constraint is that of the
parent table and the child constraint is that of the child table.
postgres=*# ALTER TABLE foo DETACH PARTITION foo_p0 ;
ALTER TABLE
postgres=*# ROLLBACK;
ROLLBACK
^
postgres=# select oid, conparentid, conrelid::regclass,
confrelid::regclass from pg_constraint where oid = 17455;
oid | conparentid | conrelid | confrelid
-------+-------------+----------+-----------
17455 | 17447 | foo_p0 | bar
(1 row)
postgres=# select oid, conparentid, conrelid::regclass,
confrelid::regclass from pg_constraint where oid = 17447;
oid | conparentid | conrelid | confrelid
-------+-------------+----------+-----------
17447 | 0 | foo | bar
(1 row
If the relation on the parent and child constraint match, that
tells us we don't have inheritance.
So, I am thinking we should add another condition for checking
if a foreign key is inherited by checking if the parent constraint
relation is different from the child constraint relation.
I am attaching an unpolished patch ( we need test coverage as well ) that
implements the above. All tests pass with this patch.
Regards,
Sami Imseih
Amazon Web Services (AWS)
[0] https://www.postgresql.org/message-id/flat/CAHewXNm5rtfQZNv2uWkiHZVJeicFFa4x7p0%3Dy-x2vAM0vorgNQ%40m...
Attachments:
[application/octet-stream] Fix-DETACH-PARTITION-with-foreign-key-referencing.patch (1.8K, ../CAA5RZ0vk4SJ9PiD2RyG-CKOYvOFewz6QweKcp2_EegBKns=dOA@mail.gmail.com/2-Fix-DETACH-PARTITION-with-foreign-key-referencing.patch)
download | inline diff:
From f3cad6887230cc20f2b4e74ed9c9f25c2b47eda1 Mon Sep 17 00:00:00 2001
From: Sami Imseih <[email protected]>
Date: Fri, 17 Jan 2025 14:25:52 -0600
Subject: [PATCH v1 1/1] Fix DETACH PARTITION with foreign key referencing a
parent partition
---
src/backend/commands/tablecmds.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d2420a9558..f2a14f0785 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20009,18 +20009,32 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
{
ForeignKeyCacheInfo *fk = lfirst(cell);
HeapTuple contup;
+ HeapTuple contup_parent;
Form_pg_constraint conform;
+ Form_pg_constraint conform_parent;
Oid insertTriggerOid,
updateTriggerOid;
+ Oid parentConRelid = InvalidOid;
contup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(fk->conoid));
if (!HeapTupleIsValid(contup))
elog(ERROR, "cache lookup failed for constraint %u", fk->conoid);
conform = (Form_pg_constraint) GETSTRUCT(contup);
+ if (OidIsValid(conform->conparentid))
+ {
+ contup_parent = SearchSysCache1(CONSTROID, ObjectIdGetDatum(conform->conparentid));
+ if (!HeapTupleIsValid(contup_parent))
+ elog(ERROR, "cache lookup failed for constraint %u", conform->conparentid);
+ conform_parent = (Form_pg_constraint) GETSTRUCT(contup_parent);
+ parentConRelid = conform_parent->conrelid;
+ ReleaseSysCache(contup_parent);
+ }
+
/* consider only the inherited foreign keys */
if (conform->contype != CONSTRAINT_FOREIGN ||
- !OidIsValid(conform->conparentid))
+ !OidIsValid(conform->conparentid) ||
+ (conform->conrelid == parentConRelid))
{
ReleaseSysCache(contup);
continue;
--
2.39.5 (Apple Git-154)
view thread (3+ messages)
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]
Subject: Re: Bug in detaching a partition with a foreign key.
In-Reply-To: <CAA5RZ0vk4SJ9PiD2RyG-CKOYvOFewz6QweKcp2_EegBKns=dOA@mail.gmail.com>
* 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