From 0a2a5e92437475162161837001c7aef16f57908e Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Tue, 20 Jul 2021 16:00:31 -0400
Subject: [PATCH v9] Make ALTER TRIGGER recurse to partitions

---
 src/backend/commands/trigger.c         | 197 +++++++++++++++++++------
 src/backend/parser/gram.y              |   2 +-
 src/test/regress/expected/triggers.out | 109 ++++++++++++++
 src/test/regress/sql/triggers.sql      |  59 ++++++++
 4 files changed, 323 insertions(+), 44 deletions(-)

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 6d4b7ee92a..0005cf657d 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,17 @@ 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, unless
+	 * caller requested not to.  Lock all tables upfront, if needed.
 	 */
+	if (stmt->relation->inh &&
+		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 +1474,27 @@ 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);
+		/* Rename the trigger on this relation ... */
+		renametrig_internal(tgrel, targetrel, tuple, stmt->newname,
+							stmt->subname);
 
-		CatalogTupleUpdate(tgrel, &tuple->t_self, tuple);
+		/* ... and if it is partitioned, recurse to its partitions */
+		if (stmt->relation->inh &&
+			targetrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+		{
+			PartitionDesc partdesc = RelationGetPartitionDesc(targetrel, true);
 
-		InvokeObjectPostAlterHook(TriggerRelationId,
-								  tgoid, 0);
+			for (int i = 0; i < partdesc->nparts; i++)
+			{
+				Oid			partitionId = partdesc->oids[i];
 
-		/*
-		 * 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);
+				renametrig_partition(tgrel, partitionId, trigform->oid,
+									 stmt->newname, stmt->subname);
+			}
+		}
 	}
 	else
 	{
@@ -1533,6 +1518,132 @@ 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 newtgform;
+	ScanKeyData key[2];
+	SysScanDesc tgscan;
+
+	/*
+	 * 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 */
+	newtgform = (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(newtgform->tgname), expected_name) != 0)
+		ereport(NOTICE,
+				errmsg("renamed trigger \"%s\" on relation \"%s\"",
+					   NameStr(newtgform->tgname),
+					   RelationGetRelationName(targetrel)));
+
+	namestrcpy(&newtgform->tgname, newname);
+
+	CatalogTupleUpdate(tgrel, &tuple->t_self, tuple);
+
+	InvokeObjectPostAlterHook(TriggerRelationId, newtgform->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/backend/parser/gram.y b/src/backend/parser/gram.y
index 10da5c5c51..26433c529f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8886,7 +8886,7 @@ RenameStmt: ALTER AGGREGATE aggregate_with_argtypes RENAME TO name
 					n->missing_ok = false;
 					$$ = (Node *)n;
 				}
-			| ALTER TRIGGER name ON qualified_name RENAME TO name
+			| ALTER TRIGGER name ON relation_expr RENAME TO name
 				{
 					RenameStmt *n = makeNode(RenameStmt);
 					n->renameType = OBJECT_TRIGGER;
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index 5254447cf8..9e47bc4d45 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -3410,3 +3410,112 @@ 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 view ourtriggers as
+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;
+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 (8);
+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();
+select * from ourtriggers;
+   tgrelid   | tgname | parent_tgname 
+-------------+--------+---------------
+ chi         | a      | a
+ cho         | a      | a
+ grandparent | a      | 
+ middle      | a      | a
+(4 rows)
+
+alter trigger a on grandparent rename to b;
+select * from ourtriggers;
+   tgrelid   | tgname | parent_tgname 
+-------------+--------+---------------
+ chi         | b      | b
+ cho         | b      | b
+ grandparent | b      | 
+ middle      | b      | b
+(4 rows)
+
+alter trigger b on only middle rename to something;
+select * from ourtriggers;
+   tgrelid   |  tgname   | parent_tgname 
+-------------+-----------+---------------
+ chi         | b         | something
+ cho         | b         | something
+ grandparent | b         | 
+ middle      | something | b
+(4 rows)
+
+alter trigger something on middle rename to a_trigger;
+NOTICE:  renamed trigger "b" on relation "chi"
+NOTICE:  renamed trigger "b" on relation "cho"
+select * from ourtriggers;
+   tgrelid   |  tgname   | parent_tgname 
+-------------+-----------+---------------
+ chi         | a_trigger | a_trigger
+ cho         | a_trigger | a_trigger
+ middle      | a_trigger | b
+ grandparent | b         | 
+(4 rows)
+
+-- Renaming fails if a trigger with the target name exists in a partition
+create trigger final after insert on cho for each row execute procedure f();
+alter trigger b on grandparent rename to final;
+NOTICE:  renamed trigger "a_trigger" on relation "middle"
+ERROR:  trigger "final" for relation "cho" already exists
+select * from ourtriggers;
+   tgrelid   |  tgname   | parent_tgname 
+-------------+-----------+---------------
+ chi         | a_trigger | a_trigger
+ cho         | a_trigger | a_trigger
+ middle      | a_trigger | b
+ grandparent | b         | 
+ cho         | final     | 
+(5 rows)
+
+-- Renaming 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 * from ourtriggers;
+   tgrelid   |  tgname   | parent_tgname 
+-------------+-----------+---------------
+ chi         | a_trigger | a_trigger
+ cho         | a_trigger | a_trigger
+ middle      | a_trigger | b
+ grandparent | b         | 
+ cho         | final     | 
+ middle      | p         | 
+ grandparent | q         | 
+(7 rows)
+
+drop view ourtriggers;
+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..41e5172648 100644
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -2572,3 +2572,62 @@ 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 view ourtriggers as
+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;
+
+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 (8);
+
+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();
+
+select * from ourtriggers;
+
+alter trigger a on grandparent rename to b;
+select * from ourtriggers;
+
+alter trigger b on only middle rename to something;
+select * from ourtriggers;
+
+alter trigger something on middle rename to a_trigger;
+select * from ourtriggers;
+
+-- Renaming fails if a trigger with the target name exists in a partition
+create trigger final after insert on cho for each row execute procedure f();
+alter trigger b on grandparent rename to final;
+select * from ourtriggers;
+
+-- Renaming 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 * from ourtriggers;
+
+drop view ourtriggers;
+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();
-- 
2.30.2

