Skip to content

Commit e26c8a6

Browse files
alvherrealvherre
committed
Fix detaching partitions with cloned row triggers
When a partition is detached, any triggers that had been cloned from its parent were not properly disentangled from its parent triggers. This resulted in triggers that could not be dropped because they depended on the trigger in the trigger in the no-longer-parent table: ALTER TABLE t DETACH PARTITION t1; DROP TRIGGER trig ON t1; ERROR: cannot drop trigger trig on table t1 because trigger trig on table t requires it HINT: You can drop trigger trig on table t instead. Moreover the table can no longer be re-attached to its parent, because the trigger name is already taken: ALTER TABLE t ATTACH PARTITION t1 FOR VALUES FROM (1)TO(2); ERROR: trigger "trig" for relation "t1" already exists The former is a bug introduced in commit 86f5759. (The latter is not necessarily a bug, but it makes the bug more uncomfortable.) To avoid the complexity that would be needed to tell whether the trigger has a local definition that has to be merged with the one coming from the parent table, establish the behavior that the trigger is removed when the table is detached. Backpatch to pg11. Author: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Amit Langote <amitlangote09@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://www.postgresql.org/message-id/flat/20200408152412.GZ2228@telsasoft.com
1 parent 71ae1a8 commit e26c8a6

File tree

4 files changed

+131
-0
lines changed

4 files changed

+131
-0
lines changed

‎doc/src/sgml/ref/create_trigger.sgml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,7 @@ UPDATE OF <replaceable>column_name1</replaceable> [, <replaceable>column_name2</
526526
Creating a row-level trigger on a partitioned table will cause identical
527527
triggers to be created in all its existing partitions; and any partitions
528528
created or attached later will contain an identical trigger, too.
529+
If the partition is detached from its parent, the trigger is removed.
529530
Triggers on partitioned tables may only be <literal>AFTER</literal>.
530531
</para>
531532

‎src/backend/commands/tablecmds.c

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,7 @@ static void QueuePartitionConstraintValidation(List **wqueue, Relation scanrel,
527527
List *partConstraint,
528528
bool validate_default);
529529
static void CloneRowTriggersToPartition(Relation parent, Relation partition);
530+
static void DropClonedTriggersFromPartition(Oid partitionId);
530531
static ObjectAddress ATExecDetachPartition(Relation rel, RangeVar *name);
531532
static ObjectAddress ATExecAttachPartitionIdx(List **wqueue, Relation rel,
532533
RangeVar *name);
@@ -16439,6 +16440,9 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
1643916440
}
1644016441
table_close(classRel, RowExclusiveLock);
1644116442

16443+
/* Drop any triggers that were cloned on creation/attach. */
16444+
DropClonedTriggersFromPartition(RelationGetRelid(partRel));
16445+
1644216446
/*
1644316447
* Detach any foreign keys that are inherited. This includes creating
1644416448
* additional action triggers.
@@ -16523,6 +16527,66 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
1652316527
return address;
1652416528
}
1652516529

16530+
/*
16531+
* DropClonedTriggersFromPartition
16532+
* subroutine for ATExecDetachPartition to remove any triggers that were
16533+
* cloned to the partition when it was created-as-partition or attached.
16534+
* This undoes what CloneRowTriggersToPartition did.
16535+
*/
16536+
static void
16537+
DropClonedTriggersFromPartition(Oid partitionId)
16538+
{
16539+
ScanKeyData skey;
16540+
SysScanDesc scan;
16541+
HeapTuple trigtup;
16542+
Relation tgrel;
16543+
ObjectAddresses *objects;
16544+
16545+
objects = new_object_addresses();
16546+
16547+
/*
16548+
* Scan pg_trigger to search for all triggers on this rel.
16549+
*/
16550+
ScanKeyInit(&skey, Anum_pg_trigger_tgrelid, BTEqualStrategyNumber,
16551+
F_OIDEQ, ObjectIdGetDatum(partitionId));
16552+
tgrel = table_open(TriggerRelationId, RowExclusiveLock);
16553+
scan = systable_beginscan(tgrel, TriggerRelidNameIndexId,
16554+
true, NULL, 1, &skey);
16555+
while (HeapTupleIsValid(trigtup = systable_getnext(scan)))
16556+
{
16557+
Form_pg_trigger pg_trigger = (Form_pg_trigger) GETSTRUCT(trigtup);
16558+
ObjectAddress trig;
16559+
16560+
/* Ignore triggers that weren't cloned */
16561+
if (!isPartitionTrigger(pg_trigger->oid))
16562+
continue;
16563+
16564+
/*
16565+
* This is ugly, but necessary: remove the dependency markings on the
16566+
* trigger so that it can be removed.
16567+
*/
16568+
deleteDependencyRecordsForClass(TriggerRelationId, pg_trigger->oid,
16569+
TriggerRelationId,
16570+
DEPENDENCY_PARTITION_PRI);
16571+
deleteDependencyRecordsForClass(TriggerRelationId, pg_trigger->oid,
16572+
RelationRelationId,
16573+
DEPENDENCY_PARTITION_SEC);
16574+
16575+
/* remember this trigger to remove it below */
16576+
ObjectAddressSet(trig, TriggerRelationId, pg_trigger->oid);
16577+
add_exact_object_address(&trig, objects);
16578+
}
16579+
16580+
/* make the dependency removal visible to the deletion below */
16581+
CommandCounterIncrement();
16582+
performMultipleDeletions(objects, DROP_RESTRICT, PERFORM_DELETION_INTERNAL);
16583+
16584+
/* done */
16585+
free_object_addresses(objects);
16586+
systable_endscan(scan);
16587+
table_close(tgrel, RowExclusiveLock);
16588+
}
16589+
1652616590
/*
1652716591
* Before acquiring lock on an index, acquire the same lock on the owning
1652816592
* table.

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2027,6 +2027,51 @@ select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger
20272027
---------+--------+--------
20282028
(0 rows)
20292029

2030+
-- check detach behavior
2031+
create trigger trg1 after insert on trigpart for each row execute procedure trigger_nothing();
2032+
\d trigpart3
2033+
Table "public.trigpart3"
2034+
Column | Type | Collation | Nullable | Default
2035+
--------+---------+-----------+----------+---------
2036+
a | integer | | |
2037+
b | integer | | |
2038+
Partition of: trigpart FOR VALUES FROM (2000) TO (3000)
2039+
Triggers:
2040+
trg1 AFTER INSERT ON trigpart3 FOR EACH ROW EXECUTE FUNCTION trigger_nothing()
2041+
2042+
alter table trigpart detach partition trigpart3;
2043+
drop trigger trg1 on trigpart3; -- fail due to "does not exist"
2044+
ERROR: trigger "trg1" for table "trigpart3" does not exist
2045+
alter table trigpart detach partition trigpart4;
2046+
drop trigger trg1 on trigpart41; -- fail due to "does not exist"
2047+
ERROR: trigger "trg1" for table "trigpart41" does not exist
2048+
drop table trigpart4;
2049+
alter table trigpart attach partition trigpart3 for values from (2000) to (3000);
2050+
alter table trigpart detach partition trigpart3;
2051+
alter table trigpart attach partition trigpart3 for values from (2000) to (3000);
2052+
drop table trigpart3;
2053+
select tgrelid::regclass::text, tgname, tgfoid::regproc, tgenabled, tgisinternal from pg_trigger
2054+
where tgname ~ '^trg1' order by 1;
2055+
tgrelid | tgname | tgfoid | tgenabled | tgisinternal
2056+
-----------+--------+-----------------+-----------+--------------
2057+
trigpart | trg1 | trigger_nothing | O | f
2058+
trigpart1 | trg1 | trigger_nothing | O | t
2059+
(2 rows)
2060+
2061+
create table trigpart3 (like trigpart);
2062+
create trigger trg1 after insert on trigpart3 for each row execute procedure trigger_nothing();
2063+
\d trigpart3
2064+
Table "public.trigpart3"
2065+
Column | Type | Collation | Nullable | Default
2066+
--------+---------+-----------+----------+---------
2067+
a | integer | | |
2068+
b | integer | | |
2069+
Triggers:
2070+
trg1 AFTER INSERT ON trigpart3 FOR EACH ROW EXECUTE FUNCTION trigger_nothing()
2071+
2072+
alter table trigpart attach partition trigpart3 FOR VALUES FROM (2000) to (3000); -- fail
2073+
ERROR: trigger "trg1" for relation "trigpart3" already exists
2074+
drop table trigpart3;
20302075
drop table trigpart;
20312076
drop function trigger_nothing();
20322077
--

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1382,6 +1382,27 @@ drop trigger trg1 on trigpart; -- ok, all gone
13821382
select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger
13831383
where tgrelid::regclass::text like 'trigpart%' order by tgrelid::regclass::text;
13841384

1385+
-- check detach behavior
1386+
create trigger trg1 after insert on trigpart for each row execute procedure trigger_nothing();
1387+
\d trigpart3
1388+
alter table trigpart detach partition trigpart3;
1389+
drop trigger trg1 on trigpart3; -- fail due to "does not exist"
1390+
alter table trigpart detach partition trigpart4;
1391+
drop trigger trg1 on trigpart41; -- fail due to "does not exist"
1392+
drop table trigpart4;
1393+
alter table trigpart attach partition trigpart3 for values from (2000) to (3000);
1394+
alter table trigpart detach partition trigpart3;
1395+
alter table trigpart attach partition trigpart3 for values from (2000) to (3000);
1396+
drop table trigpart3;
1397+
1398+
select tgrelid::regclass::text, tgname, tgfoid::regproc, tgenabled, tgisinternal from pg_trigger
1399+
where tgname ~ '^trg1' order by 1;
1400+
create table trigpart3 (like trigpart);
1401+
create trigger trg1 after insert on trigpart3 for each row execute procedure trigger_nothing();
1402+
\d trigpart3
1403+
alter table trigpart attach partition trigpart3 FOR VALUES FROM (2000) to (3000); -- fail
1404+
drop table trigpart3;
1405+
13851406
drop table trigpart;
13861407
drop function trigger_nothing();
13871408

0 commit comments

Comments
 (0)