diff --git a/doc/src/sgml/ref/alter_trigger.sgml b/doc/src/sgml/ref/alter_trigger.sgml index 43a7da4f0b..ece9cb5acf 100644 --- a/doc/src/sgml/ref/alter_trigger.sgml +++ b/doc/src/sgml/ref/alter_trigger.sgml @@ -31,9 +31,20 @@ ALTER TRIGGER name ON ALTER TRIGGER changes properties of an existing - trigger. The RENAME clause changes the name of + trigger. + + + + The RENAME clause changes the name of the given trigger without otherwise changing the trigger - definition. The DEPENDS ON EXTENSION clause marks + definition. + If the table that the trigger is on is a partitioned table, + then corresponding clone triggers in the partitions are + renamed too. + + + + The DEPENDS ON EXTENSION clause marks the trigger as dependent on an extension, such that if the extension is dropped, the trigger will automatically be dropped as well. diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 6d4b7ee92a..fc0a4b2fa7 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -71,6 +71,12 @@ int SessionReplicationRole = SESSION_REPLICATION_ROLE_ORIGIN; static int MyTriggerDepth = 0; /* Local function prototypes */ +static void renametrig_internal(Relation tgrel, Relation targetrel, + HeapTuple trigtup, const char *newname, + const char *expected_name); +static void renametrig_partition(Relation tgrel, Oid partitionId, + Oid parentTriggerOid, const char *newname, + const char *expected_name); static void SetTriggerFlags(TriggerDesc *trigdesc, Trigger *trigger); static bool GetTupleForTrigger(EState *estate, EPQState *epqstate, @@ -1442,38 +1448,16 @@ renametrig(RenameStmt *stmt) targetrel = relation_open(relid, NoLock); /* - * Scan pg_trigger twice for existing triggers on relation. We do this in - * order to ensure a trigger does not exist with newname (The unique index - * on tgrelid/tgname would complain anyway) and to ensure a trigger does - * exist with oldname. - * - * NOTE that this is cool only because we have AccessExclusiveLock on the - * relation, so the trigger set won't be changing underneath us. + * On partitioned tables, this operation recurses to partitions. Lock all + * tables upfront. */ + if (targetrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + (void) find_all_inheritors(relid, AccessExclusiveLock, NULL); + tgrel = table_open(TriggerRelationId, RowExclusiveLock); /* - * First pass -- look for name conflict - */ - ScanKeyInit(&key[0], - Anum_pg_trigger_tgrelid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(relid)); - ScanKeyInit(&key[1], - Anum_pg_trigger_tgname, - BTEqualStrategyNumber, F_NAMEEQ, - PointerGetDatum(stmt->newname)); - tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true, - NULL, 2, key); - if (HeapTupleIsValid(tuple = systable_getnext(tgscan))) - ereport(ERROR, - (errcode(ERRCODE_DUPLICATE_OBJECT), - errmsg("trigger \"%s\" for relation \"%s\" already exists", - stmt->newname, RelationGetRelationName(targetrel)))); - systable_endscan(tgscan); - - /* - * Second pass -- look for trigger existing with oldname and update + * Search for the trigger to modify. */ ScanKeyInit(&key[0], Anum_pg_trigger_tgrelid, @@ -1489,27 +1473,40 @@ renametrig(RenameStmt *stmt) { Form_pg_trigger trigform; - /* - * Update pg_trigger tuple with new tgname. - */ - tuple = heap_copytuple(tuple); /* need a modifiable copy */ trigform = (Form_pg_trigger) GETSTRUCT(tuple); tgoid = trigform->oid; - namestrcpy(&trigform->tgname, - stmt->newname); - - CatalogTupleUpdate(tgrel, &tuple->t_self, tuple); - - InvokeObjectPostAlterHook(TriggerRelationId, - tgoid, 0); - /* - * Invalidate relation's relcache entry so that other backends (and - * this one too!) are sent SI message to make them rebuild relcache - * entries. (Ideally this should happen automatically...) + * If the trigger descends from a trigger on a parent partitioned + * table, reject the rename. We don't allow a trigger in a partition + * to differ in name from that of its parent: that would lead to an + * inconsistency that pg_dump would not reproduce. */ - CacheInvalidateRelcache(targetrel); + if (OidIsValid(trigform->tgparentid)) + ereport(ERROR, + errmsg("cannot rename trigger \"%s\" on table \"%s\"", + stmt->subname, RelationGetRelationName(targetrel)), + errhint("Rename trigger on partitioned table \"%s\" instead.", + get_rel_name(get_partition_parent(relid, false)))); + + + /* Rename the trigger on this relation ... */ + renametrig_internal(tgrel, targetrel, tuple, stmt->newname, + stmt->subname); + + /* ... and if it is partitioned, recurse to its partitions */ + if (targetrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + { + PartitionDesc partdesc = RelationGetPartitionDesc(targetrel, true); + + for (int i = 0; i < partdesc->nparts; i++) + { + Oid partitionId = partdesc->oids[i]; + + renametrig_partition(tgrel, partitionId, trigform->oid, + stmt->newname, stmt->subname); + } + } } else { @@ -1533,6 +1530,137 @@ renametrig(RenameStmt *stmt) return address; } +/* + * Subroutine for renametrig -- perform the actual work of renaming one + * trigger on one table. + * + * If the trigger has a name different from the expected one, raise a + * NOTICE about it. + */ +static void +renametrig_internal(Relation tgrel, Relation targetrel, HeapTuple trigtup, + const char *newname, const char *expected_name) +{ + HeapTuple tuple; + Form_pg_trigger tgform; + ScanKeyData key[2]; + SysScanDesc tgscan; + + /* If the trigger already has the new name, nothing to do. */ + tgform = (Form_pg_trigger) GETSTRUCT(trigtup); + if (strcmp(NameStr(tgform->tgname), newname) == 0) + return; + + /* + * Before actually trying the rename, search for triggers with the same + * name. The update would fail with an ugly message in that case, and it + * is better to throw a nicer error. + */ + ScanKeyInit(&key[0], + Anum_pg_trigger_tgrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationGetRelid(targetrel))); + ScanKeyInit(&key[1], + Anum_pg_trigger_tgname, + BTEqualStrategyNumber, F_NAMEEQ, + PointerGetDatum(newname)); + tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true, + NULL, 2, key); + if (HeapTupleIsValid(tuple = systable_getnext(tgscan))) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("trigger \"%s\" for relation \"%s\" already exists", + newname, RelationGetRelationName(targetrel)))); + systable_endscan(tgscan); + + /* + * The target name is free; update the existing pg_trigger tuple with it. + */ + tuple = heap_copytuple(trigtup); /* need a modifiable copy */ + tgform = (Form_pg_trigger) GETSTRUCT(tuple); + + /* + * If the trigger has a name different from what we expected, let the user + * know. (We can proceed anyway, since we must have reached here following + * a tgparentid link.) + */ + if (strcmp(NameStr(tgform->tgname), expected_name) != 0) + ereport(NOTICE, + errmsg("renamed trigger \"%s\" on relation \"%s\"", + NameStr(tgform->tgname), + RelationGetRelationName(targetrel))); + + namestrcpy(&tgform->tgname, newname); + + CatalogTupleUpdate(tgrel, &tuple->t_self, tuple); + + InvokeObjectPostAlterHook(TriggerRelationId, tgform->oid, 0); + + /* + * Invalidate relation's relcache entry so that other backends (and this + * one too!) are sent SI message to make them rebuild relcache entries. + * (Ideally this should happen automatically...) + */ + CacheInvalidateRelcache(targetrel); +} + +/* + * Subroutine for renametrig -- Helper for recursing to partitions when + * renaming triggers on a partitioned table. + */ +static void +renametrig_partition(Relation tgrel, Oid partitionId, Oid parentTriggerOid, + const char *newname, const char *expected_name) +{ + SysScanDesc tgscan; + ScanKeyData key; + HeapTuple tuple; + int found PG_USED_FOR_ASSERTS_ONLY = 0; + + /* + * Given a relation and the OID of a trigger on parent relation, find the + * corresponding trigger in the child and rename that trigger to the given + * name. + */ + ScanKeyInit(&key, + Anum_pg_trigger_tgrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(partitionId)); + tgscan = systable_beginscan(tgrel, TriggerRelidNameIndexId, true, + NULL, 1, &key); + while (HeapTupleIsValid(tuple = systable_getnext(tgscan))) + { + Form_pg_trigger tgform = (Form_pg_trigger) GETSTRUCT(tuple); + Relation partitionRel; + + if (tgform->tgparentid != parentTriggerOid) + continue; /* not our trigger */ + + Assert(found++ <= 0); + + partitionRel = table_open(partitionId, NoLock); + + /* Rename the trigger on this partition */ + renametrig_internal(tgrel, partitionRel, tuple, newname, expected_name); + + /* And if this relation is partitioned, recurse to its partitions */ + if (partitionRel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + { + PartitionDesc partdesc = RelationGetPartitionDesc(partitionRel, + true); + + for (int i = 0; i < partdesc->nparts; i++) + { + Oid partitionId = partdesc->oids[i]; + + renametrig_partition(tgrel, partitionId, tgform->oid, newname, + NameStr(tgform->tgname)); + } + } + table_close(partitionRel, NoLock); + } + systable_endscan(tgscan); +} /* * EnableDisableTrigger() diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out index 5254447cf8..564eb4faa2 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -3410,3 +3410,79 @@ for each statement execute function trigger_function1(); delete from convslot_test_parent; NOTICE: trigger = bdt_trigger, old_table = (111,tutu), (311,tutu) drop table convslot_test_child, convslot_test_parent; +-- Test trigger renaming on partitioned tables +create table grandparent (id int, primary key (id)) partition by range (id); +create table middle partition of grandparent for values from (1) to (10) +partition by range (id); +create table chi partition of middle for values from (1) to (5); +create table cho partition of middle for values from (6) to (10); +create function f () returns trigger as +$$ begin return new; end; $$ +language plpgsql; +create trigger a after insert on grandparent +for each row execute procedure f(); +alter trigger a on grandparent rename to b; +select tgrelid::regclass, tgname, +(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname +from pg_trigger where tgrelid in (select relid from pg_partition_tree('grandparent')) +order by tgname, tgrelid::regclass::text; + tgrelid | tgname | parent_tgname +-------------+--------+--------------- + chi | b | b + cho | b | b + grandparent | b | + middle | b | b +(4 rows) + +alter trigger a on only grandparent rename to b; -- ONLY not supported +ERROR: syntax error at or near "only" +LINE 1: alter trigger a on only grandparent rename to b; + ^ +alter trigger b on middle rename to c; -- can't rename trigger on partition +ERROR: cannot rename trigger "b" on table "middle" +HINT: Rename trigger on partitioned table "grandparent" instead. +create trigger c after insert on middle +for each row execute procedure f(); +alter trigger b on grandparent rename to c; +ERROR: trigger "c" for relation "middle" already exists +-- Rename cascading does not affect statement triggers +create trigger p after insert on grandparent for each statement execute function f(); +create trigger p after insert on middle for each statement execute function f(); +alter trigger p on grandparent rename to q; +select tgrelid::regclass, tgname, +(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname +from pg_trigger where tgrelid in (select relid from pg_partition_tree('grandparent')) +order by tgname, tgrelid::regclass::text; + tgrelid | tgname | parent_tgname +-------------+--------+--------------- + chi | b | b + cho | b | b + grandparent | b | + middle | b | b + chi | c | c + cho | c | c + middle | c | + middle | p | + grandparent | q | +(9 rows) + +drop table grandparent; +-- Trigger renaming does not recurse on legacy inheritance +create table parent (a int); +create table child () inherits (parent); +create trigger parenttrig after insert on parent +for each row execute procedure f(); +create trigger parenttrig after insert on child +for each row execute procedure f(); +alter trigger parenttrig on parent rename to anothertrig; +\d+ child + Table "public.child" + Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------+---------+--------------+------------- + a | integer | | | | plain | | +Triggers: + parenttrig AFTER INSERT ON child FOR EACH ROW EXECUTE FUNCTION f() +Inherits: parent + +drop table parent, child; +drop function f(); diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql index 7b73ee20a1..fb94eca3ed 100644 --- a/src/test/regress/sql/triggers.sql +++ b/src/test/regress/sql/triggers.sql @@ -2572,3 +2572,50 @@ for each statement execute function trigger_function1(); delete from convslot_test_parent; drop table convslot_test_child, convslot_test_parent; + +-- Test trigger renaming on partitioned tables +create table grandparent (id int, primary key (id)) partition by range (id); +create table middle partition of grandparent for values from (1) to (10) +partition by range (id); +create table chi partition of middle for values from (1) to (5); +create table cho partition of middle for values from (6) to (10); +create function f () returns trigger as +$$ begin return new; end; $$ +language plpgsql; +create trigger a after insert on grandparent +for each row execute procedure f(); + +alter trigger a on grandparent rename to b; +select tgrelid::regclass, tgname, +(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname +from pg_trigger where tgrelid in (select relid from pg_partition_tree('grandparent')) +order by tgname, tgrelid::regclass::text; +alter trigger a on only grandparent rename to b; -- ONLY not supported +alter trigger b on middle rename to c; -- can't rename trigger on partition +create trigger c after insert on middle +for each row execute procedure f(); +alter trigger b on grandparent rename to c; + +-- Rename cascading does not affect statement triggers +create trigger p after insert on grandparent for each statement execute function f(); +create trigger p after insert on middle for each statement execute function f(); +alter trigger p on grandparent rename to q; +select tgrelid::regclass, tgname, +(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname +from pg_trigger where tgrelid in (select relid from pg_partition_tree('grandparent')) +order by tgname, tgrelid::regclass::text; + +drop table grandparent; + +-- Trigger renaming does not recurse on legacy inheritance +create table parent (a int); +create table child () inherits (parent); +create trigger parenttrig after insert on parent +for each row execute procedure f(); +create trigger parenttrig after insert on child +for each row execute procedure f(); +alter trigger parenttrig on parent rename to anothertrig; +\d+ child + +drop table parent, child; +drop function f();