Skip to content

Commit 1fa846f

Browse files
alvherrealvherre
committed
Fix cloning of row triggers to sub-partitions
When row triggers exist in partitioned partitions that are not either part of FKs or deferred unique constraints, they are not correctly cloned to their partitions. That's because they are marked "internal", and those are purposefully skipped when doing the clone triggers dance. Fix by relaxing the condition on which internal triggers are skipped. Amit Langote initially diagnosed the problem and proposed a fix, but I used a different approach. Reported-by: Petr Fedorov Discussion: https://postgr.es/m/6b3f0646-ba8c-b3a9-c62d-1c6651a1920f@phystech.edu
1 parent 7c01504 commit 1fa846f

File tree

3 files changed

+90
-15
lines changed

3 files changed

+90
-15
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15931,6 +15931,54 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel)
1593115931
MemoryContextDelete(cxt);
1593215932
}
1593315933

15934+
/*
15935+
* isPartitionTrigger
15936+
* Subroutine for CloneRowTriggersToPartition: determine whether
15937+
* the given trigger has been cloned from another one.
15938+
*
15939+
* We use pg_depend as a proxy for this, since we don't have any direct
15940+
* evidence. This is an ugly hack to cope with a catalog deficiency.
15941+
* Keep away from children. Do not stare with naked eyes. Do not propagate.
15942+
*/
15943+
static bool
15944+
isPartitionTrigger(Oid trigger_oid)
15945+
{
15946+
Relation pg_depend;
15947+
ScanKeyData key[2];
15948+
SysScanDesc scan;
15949+
HeapTuple tup;
15950+
bool found = false;
15951+
15952+
pg_depend = table_open(DependRelationId, AccessShareLock);
15953+
15954+
ScanKeyInit(&key[0], Anum_pg_depend_classid,
15955+
BTEqualStrategyNumber,
15956+
F_OIDEQ,
15957+
ObjectIdGetDatum(TriggerRelationId));
15958+
ScanKeyInit(&key[1], Anum_pg_depend_objid,
15959+
BTEqualStrategyNumber,
15960+
F_OIDEQ,
15961+
ObjectIdGetDatum(trigger_oid));
15962+
15963+
scan = systable_beginscan(pg_depend, DependDependerIndexId,
15964+
true, NULL, 2, key);
15965+
while ((tup = systable_getnext(scan)) != NULL)
15966+
{
15967+
Form_pg_depend dep = (Form_pg_depend) GETSTRUCT(tup);
15968+
15969+
if (dep->refclassid == TriggerRelationId)
15970+
{
15971+
found = true;
15972+
break;
15973+
}
15974+
}
15975+
15976+
systable_endscan(scan);
15977+
table_close(pg_depend, AccessShareLock);
15978+
15979+
return found;
15980+
}
15981+
1593415982
/*
1593515983
* CloneRowTriggersToPartition
1593615984
* subroutine for ATExecAttachPartition/DefineRelation to create row
@@ -15971,8 +16019,21 @@ CloneRowTriggersToPartition(Relation parent, Relation partition)
1597116019
if (!TRIGGER_FOR_ROW(trigForm->tgtype))
1597216020
continue;
1597316021

15974-
/* We don't clone internal triggers, either */
15975-
if (trigForm->tgisinternal)
16022+
/*
16023+
* Internal triggers require careful examination. Ideally, we don't
16024+
* clone them.
16025+
*
16026+
* However, if our parent is a partitioned relation, there might be
16027+
* internal triggers that need cloning. In that case, we must
16028+
* skip clone it if the trigger on parent depends on another trigger.
16029+
*
16030+
* Note we dare not verify that the other trigger belongs to an
16031+
* ancestor relation of our parent, because that creates deadlock
16032+
* opportunities.
16033+
*/
16034+
if (trigForm->tgisinternal &&
16035+
(!parent->rd_rel->relispartition ||
16036+
!isPartitionTrigger(trigForm->oid)))
1597616037
continue;
1597716038

1597816039
/*

‎src/test/regress/expected/triggers.out

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1981,15 +1981,22 @@ create trigger trg1 after insert on trigpart for each row execute procedure trig
19811981
create table trigpart2 partition of trigpart for values from (1000) to (2000);
19821982
create table trigpart3 (like trigpart);
19831983
alter table trigpart attach partition trigpart3 for values from (2000) to (3000);
1984+
create table trigpart4 partition of trigpart for values from (3000) to (4000) partition by range (a);
1985+
create table trigpart41 partition of trigpart4 for values from (3000) to (3500);
1986+
create table trigpart42 (like trigpart);
1987+
alter table trigpart4 attach partition trigpart42 for values from (3500) to (4000);
19841988
select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger
19851989
where tgrelid::regclass::text like 'trigpart%' order by tgrelid::regclass::text;
1986-
tgrelid | tgname | tgfoid
1987-
-----------+--------+-----------------
1988-
trigpart | trg1 | trigger_nothing
1989-
trigpart1 | trg1 | trigger_nothing
1990-
trigpart2 | trg1 | trigger_nothing
1991-
trigpart3 | trg1 | trigger_nothing
1992-
(4 rows)
1990+
tgrelid | tgname | tgfoid
1991+
------------+--------+-----------------
1992+
trigpart | trg1 | trigger_nothing
1993+
trigpart1 | trg1 | trigger_nothing
1994+
trigpart2 | trg1 | trigger_nothing
1995+
trigpart3 | trg1 | trigger_nothing
1996+
trigpart4 | trg1 | trigger_nothing
1997+
trigpart41 | trg1 | trigger_nothing
1998+
trigpart42 | trg1 | trigger_nothing
1999+
(7 rows)
19932000

19942001
drop trigger trg1 on trigpart1; -- fail
19952002
ERROR: cannot drop trigger trg1 on table trigpart1 because trigger trg1 on table trigpart requires it
@@ -2003,12 +2010,15 @@ HINT: You can drop trigger trg1 on table trigpart instead.
20032010
drop table trigpart2; -- ok, trigger should be gone in that partition
20042011
select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger
20052012
where tgrelid::regclass::text like 'trigpart%' order by tgrelid::regclass::text;
2006-
tgrelid | tgname | tgfoid
2007-
-----------+--------+-----------------
2008-
trigpart | trg1 | trigger_nothing
2009-
trigpart1 | trg1 | trigger_nothing
2010-
trigpart3 | trg1 | trigger_nothing
2011-
(3 rows)
2013+
tgrelid | tgname | tgfoid
2014+
------------+--------+-----------------
2015+
trigpart | trg1 | trigger_nothing
2016+
trigpart1 | trg1 | trigger_nothing
2017+
trigpart3 | trg1 | trigger_nothing
2018+
trigpart4 | trg1 | trigger_nothing
2019+
trigpart41 | trg1 | trigger_nothing
2020+
trigpart42 | trg1 | trigger_nothing
2021+
(6 rows)
20122022

20132023
drop trigger trg1 on trigpart; -- ok, all gone
20142024
select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger

‎src/test/regress/sql/triggers.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,6 +1366,10 @@ create trigger trg1 after insert on trigpart for each row execute procedure trig
13661366
create table trigpart2 partition of trigpart for values from (1000) to (2000);
13671367
create table trigpart3 (like trigpart);
13681368
alter table trigpart attach partition trigpart3 for values from (2000) to (3000);
1369+
create table trigpart4 partition of trigpart for values from (3000) to (4000) partition by range (a);
1370+
create table trigpart41 partition of trigpart4 for values from (3000) to (3500);
1371+
create table trigpart42 (like trigpart);
1372+
alter table trigpart4 attach partition trigpart42 for values from (3500) to (4000);
13691373
select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger
13701374
where tgrelid::regclass::text like 'trigpart%' order by tgrelid::regclass::text;
13711375
drop trigger trg1 on trigpart1; -- fail

0 commit comments

Comments
 (0)