From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Arne Roland <A.Roland@index.de>
Cc: vignesh C <vignesh21@gmail.com>
Cc: Zhihong Yu <zyu@yugabyte.com>
Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Rename of triggers for partitioned tables
Date: Mon, 19 Jul 2021 20:03:08 -0400
Message-ID: <202107200003.nve6a53eixra@alvherre.pgsql> (raw)
In-Reply-To: <6e6491608f694ca488a5d7c404c4b98d@index.de>
I found this coding too convoluted, so I rewrote it in a different way.
You tests pass with this, but I admit I haven't double-checked them yet;
I'll do that next.
I don't think we need to give a NOTICE when the trigger name does not
match; it doesn't really matter that the trigger was named differently
before the command, does it?
--
Álvaro Herrera 39°49'30"S 73°17'W — https://www.EnterpriseDB.com/Attachments:renametrig-8.patch (text/x-diff, 13.7K) downloaddiff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.cindex 6d4b7ee92a..da6320f6a1 100644--- a/src/backend/commands/trigger.c+++ b/src/backend/commands/trigger.c@@ -71,6 +71,10 @@ 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);+static void renametrig_partition(Relation tgrel, Oid partitionId,+ Oid parentTriggerOid, const char *newname);
static void SetTriggerFlags(TriggerDesc *trigdesc, Trigger *trigger);
static bool GetTupleForTrigger(EState *estate,
EPQState *epqstate,
@@ -1441,6 +1445,14 @@ renametrig(RenameStmt *stmt)
/* Have lock already, so just need to build relcache entry. */
targetrel = relation_open(relid, NoLock);
+ /*+ * 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);+
/*
* 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
@@ -1489,27 +1501,25 @@ 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);- 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);+ }+ }
}
else
{
@@ -1533,6 +1543,92 @@ renametrig(RenameStmt *stmt)
return address;
}
+/*+ * Subroutine for renametrig -- perform the actual work of renaming one+ * trigger on one table.+ */+static void+renametrig_internal(Relation tgrel, Relation targetrel, HeapTuple trigtup,+ const char *newname)+{+ HeapTuple tuple;+ Form_pg_trigger newtgform;++ /*+ * Update pg_trigger tuple with new tgname.+ */+ tuple = heap_copytuple(trigtup); /* need a modifiable copy */+ newtgform = (Form_pg_trigger) GETSTRUCT(tuple);++ 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)+{+ SysScanDesc tgscan;+ ScanKeyData key;+ HeapTuple tuple;+ int found = 0 PG_USED_FOR_ASSERTS_ONLY;++ /*+ * 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);++ /* 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);+ }+ }+ table_close(partitionRel, NoLock);+ }+ systable_endscan(tgscan);+}
/*
* EnableDisableTrigger()
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.yindex 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.outindex 5254447cf8..0380257aee 100644--- a/src/test/regress/expected/triggers.out+++ b/src/test/regress/expected/triggers.out@@ -3410,3 +3410,122 @@ 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;
+--+-- build a nested partition scheme for testing+--+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 (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 tgrelid::regclass, tgname,+(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname+from pg_trigger where tgname in ('a')+order by tgname, tgrelid::regclass::text;+ 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 tgrelid::regclass, tgname,+(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname+from pg_trigger where tgname in ('a', 'b')+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 b on only middle rename to something;+select tgrelid::regclass, tgname,+(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname+from pg_trigger where tgname in ('something', 'b')+order by tgname, tgrelid::regclass::text;+ 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;+select tgrelid::regclass, tgname,+(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname+from pg_trigger where tgname in ('a_trigger', 'b', 'something')+order by tgname, tgrelid::regclass::text;+ tgrelid | tgname | parent_tgname +-------------+-----------+---------------+ chi | a_trigger | a_trigger+ cho | a_trigger | a_trigger+ middle | a_trigger | b+ grandparent | b | +(4 rows)++--+-- Check that classical inheritance is unphased by renaming triggers+--+create table inh (id int,+primary key (id));+select tgrelid::regclass, tgname,+(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname+from pg_trigger where tgrelid = 'chi'::regclass+order by tgname, tgrelid::regclass::text;+ tgrelid | tgname | parent_tgname +---------+-----------+---------------+ chi | a_trigger | a_trigger+(1 row)++alter table middle detach partition chi;+alter trigger a_trigger on middle rename to some_trigger;+select tgrelid::regclass, tgname,+(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname+from pg_trigger where tgrelid = 'chi'::regclass+order by tgname, tgrelid::regclass::text;+ tgrelid | tgname | parent_tgname +---------+--------+---------------+(0 rows)++alter table chi inherit inh;+select tgrelid::regclass, tgname,+(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname+from pg_trigger where tgrelid = 'chi'::regclass+order by tgname, tgrelid::regclass::text;+ tgrelid | tgname | parent_tgname +---------+--------+---------------+(0 rows)++create trigger unrelated before update of id on inh+for each row execute procedure f();+alter trigger unrelated on inh rename to some_trigger;+alter trigger some_trigger on inh rename to trigger_name;+select tgrelid::regclass, tgname,+(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname+from pg_trigger where tgname in ('some_trigger', 'unrelated', 'trigger_name')+order by tgname, tgrelid::regclass::text;+ tgrelid | tgname | parent_tgname +---------+--------------+---------------+ cho | some_trigger | some_trigger+ middle | some_trigger | b+ inh | trigger_name | +(3 rows)++-- cleanup+drop table grandparent;+drop table chi;+drop table inh;+drop function f();diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sqlindex 7b73ee20a1..5be330508e 100644--- a/src/test/regress/sql/triggers.sql+++ b/src/test/regress/sql/triggers.sql@@ -2572,3 +2572,96 @@ for each statement execute function trigger_function1();
delete from convslot_test_parent;
drop table convslot_test_child, convslot_test_parent;
++--+-- build a nested partition scheme for testing+--+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 (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 tgrelid::regclass, tgname,+(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname+from pg_trigger where tgname in ('a')+order by tgname, tgrelid::regclass::text;++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 tgname in ('a', 'b')+order by tgname, tgrelid::regclass::text;++alter trigger b on only middle rename to something;++select tgrelid::regclass, tgname,+(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname+from pg_trigger where tgname in ('something', 'b')+order by tgname, tgrelid::regclass::text;++alter trigger something on middle rename to a_trigger;++select tgrelid::regclass, tgname,+(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname+from pg_trigger where tgname in ('a_trigger', 'b', 'something')+order by tgname, tgrelid::regclass::text;++--+-- Check that classical inheritance is unphased by renaming triggers+--++create table inh (id int,+primary key (id));++select tgrelid::regclass, tgname,+(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname+from pg_trigger where tgrelid = 'chi'::regclass+order by tgname, tgrelid::regclass::text;++alter table middle detach partition chi;++alter trigger a_trigger on middle rename to some_trigger;++select tgrelid::regclass, tgname,+(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname+from pg_trigger where tgrelid = 'chi'::regclass+order by tgname, tgrelid::regclass::text;++alter table chi inherit inh;++select tgrelid::regclass, tgname,+(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname+from pg_trigger where tgrelid = 'chi'::regclass+order by tgname, tgrelid::regclass::text;++create trigger unrelated before update of id on inh+for each row execute procedure f();++alter trigger unrelated on inh rename to some_trigger;++alter trigger some_trigger on inh rename to trigger_name;++select tgrelid::regclass, tgname,+(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname+from pg_trigger where tgname in ('some_trigger', 'unrelated', 'trigger_name')+order by tgname, tgrelid::regclass::text;++-- cleanup++drop table grandparent;+drop table chi;+drop table inh;+drop function f();
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: pgsql-hackers@postgresql.org
Cc: alvherre@alvh.no-ip.org, A.Roland@index.de, vignesh21@gmail.com, zyu@yugabyte.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Rename of triggers for partitioned tables
In-Reply-To: <202107200003.nve6a53eixra@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox