Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit80ba4bb

Browse files
alvherreTel44
andcommitted
Make ALTER TRIGGER RENAME consistent for partitioned tables
Renaming triggers on partitioned tables had two problems: first,it did not recurse to renaming the triggers on the partitions; andsecond, it failed to prohibit renaming clone triggers. Having triggerswith different names in partitions is pointless, and furthermore pg_dumpwould not preserve names for partitions anyway.Not backpatched -- making the ALTER TRIGGER throw an error in stableversions might cause problems for existing scripts.Co-authored-by: Arne Roland <A.Roland@index.de>Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org>Reviewed-by: Zhihong Yu <zyu@yugabyte.com>Discussion:https://postgr.es/m/d0fd7040c2fb4de1a111b9d9ccc456b8@index.de
1 parent73c5d2b commit80ba4bb

File tree

4 files changed

+307
-45
lines changed

4 files changed

+307
-45
lines changed

‎doc/src/sgml/ref/alter_trigger.sgml

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,20 @@ ALTER TRIGGER <replaceable class="parameter">name</replaceable> ON <replaceable
3131

3232
<para>
3333
<command>ALTER TRIGGER</command> changes properties of an existing
34-
trigger. The <literal>RENAME</literal> clause changes the name of
34+
trigger.
35+
</para>
36+
37+
<para>
38+
The <literal>RENAME</literal> clause changes the name of
3539
the given trigger without otherwise changing the trigger
36-
definition. The <literal>DEPENDS ON EXTENSION</literal> clause marks
40+
definition.
41+
If the table that the trigger is on is a partitioned table,
42+
then corresponding clone triggers in the partitions are
43+
renamed too.
44+
</para>
45+
46+
<para>
47+
The <literal>DEPENDS ON EXTENSION</literal> clause marks
3748
the trigger as dependent on an extension, such that if the extension is
3849
dropped, the trigger will automatically be dropped as well.
3950
</para>

‎src/backend/commands/trigger.c

Lines changed: 171 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,12 @@ intSessionReplicationRole = SESSION_REPLICATION_ROLE_ORIGIN;
7171
staticintMyTriggerDepth=0;
7272

7373
/* Local function prototypes */
74+
staticvoidrenametrig_internal(Relationtgrel,Relationtargetrel,
75+
HeapTupletrigtup,constchar*newname,
76+
constchar*expected_name);
77+
staticvoidrenametrig_partition(Relationtgrel,OidpartitionId,
78+
OidparentTriggerOid,constchar*newname,
79+
constchar*expected_name);
7480
staticvoidSetTriggerFlags(TriggerDesc*trigdesc,Trigger*trigger);
7581
staticboolGetTupleForTrigger(EState*estate,
7682
EPQState*epqstate,
@@ -1442,38 +1448,16 @@ renametrig(RenameStmt *stmt)
14421448
targetrel=relation_open(relid,NoLock);
14431449

14441450
/*
1445-
* Scan pg_trigger twice for existing triggers on relation. We do this in
1446-
* order to ensure a trigger does not exist with newname (The unique index
1447-
* on tgrelid/tgname would complain anyway) and to ensure a trigger does
1448-
* exist with oldname.
1449-
*
1450-
* NOTE that this is cool only because we have AccessExclusiveLock on the
1451-
* relation, so the trigger set won't be changing underneath us.
1451+
* On partitioned tables, this operation recurses to partitions. Lock all
1452+
* tables upfront.
14521453
*/
1453-
tgrel=table_open(TriggerRelationId,RowExclusiveLock);
1454+
if (targetrel->rd_rel->relkind==RELKIND_PARTITIONED_TABLE)
1455+
(void)find_all_inheritors(relid,AccessExclusiveLock,NULL);
14541456

1455-
/*
1456-
* First pass -- look for name conflict
1457-
*/
1458-
ScanKeyInit(&key[0],
1459-
Anum_pg_trigger_tgrelid,
1460-
BTEqualStrategyNumber,F_OIDEQ,
1461-
ObjectIdGetDatum(relid));
1462-
ScanKeyInit(&key[1],
1463-
Anum_pg_trigger_tgname,
1464-
BTEqualStrategyNumber,F_NAMEEQ,
1465-
PointerGetDatum(stmt->newname));
1466-
tgscan=systable_beginscan(tgrel,TriggerRelidNameIndexId, true,
1467-
NULL,2,key);
1468-
if (HeapTupleIsValid(tuple=systable_getnext(tgscan)))
1469-
ereport(ERROR,
1470-
(errcode(ERRCODE_DUPLICATE_OBJECT),
1471-
errmsg("trigger \"%s\" for relation \"%s\" already exists",
1472-
stmt->newname,RelationGetRelationName(targetrel))));
1473-
systable_endscan(tgscan);
1457+
tgrel=table_open(TriggerRelationId,RowExclusiveLock);
14741458

14751459
/*
1476-
*Second pass -- lookfor triggerexisting with oldname and update
1460+
*Searchforthetriggerto modify.
14771461
*/
14781462
ScanKeyInit(&key[0],
14791463
Anum_pg_trigger_tgrelid,
@@ -1489,27 +1473,40 @@ renametrig(RenameStmt *stmt)
14891473
{
14901474
Form_pg_triggertrigform;
14911475

1492-
/*
1493-
* Update pg_trigger tuple with new tgname.
1494-
*/
1495-
tuple=heap_copytuple(tuple);/* need a modifiable copy */
14961476
trigform= (Form_pg_trigger)GETSTRUCT(tuple);
14971477
tgoid=trigform->oid;
14981478

1499-
namestrcpy(&trigform->tgname,
1500-
stmt->newname);
1479+
/*
1480+
* If the trigger descends from a trigger on a parent partitioned
1481+
* table, reject the rename. We don't allow a trigger in a partition
1482+
* to differ in name from that of its parent: that would lead to an
1483+
* inconsistency that pg_dump would not reproduce.
1484+
*/
1485+
if (OidIsValid(trigform->tgparentid))
1486+
ereport(ERROR,
1487+
errmsg("cannot rename trigger \"%s\" on table \"%s\"",
1488+
stmt->subname,RelationGetRelationName(targetrel)),
1489+
errhint("Rename trigger on partitioned table \"%s\" instead.",
1490+
get_rel_name(get_partition_parent(relid, false))));
15011491

1502-
CatalogTupleUpdate(tgrel,&tuple->t_self,tuple);
15031492

1504-
InvokeObjectPostAlterHook(TriggerRelationId,
1505-
tgoid,0);
1493+
/* Rename the trigger on this relation ... */
1494+
renametrig_internal(tgrel,targetrel,tuple,stmt->newname,
1495+
stmt->subname);
15061496

1507-
/*
1508-
* Invalidate relation's relcache entry so that other backends (and
1509-
* this one too!) are sent SI message to make them rebuild relcache
1510-
* entries. (Ideally this should happen automatically...)
1511-
*/
1512-
CacheInvalidateRelcache(targetrel);
1497+
/* ... and if it is partitioned, recurse to its partitions */
1498+
if (targetrel->rd_rel->relkind==RELKIND_PARTITIONED_TABLE)
1499+
{
1500+
PartitionDescpartdesc=RelationGetPartitionDesc(targetrel, true);
1501+
1502+
for (inti=0;i<partdesc->nparts;i++)
1503+
{
1504+
OidpartitionId=partdesc->oids[i];
1505+
1506+
renametrig_partition(tgrel,partitionId,trigform->oid,
1507+
stmt->newname,stmt->subname);
1508+
}
1509+
}
15131510
}
15141511
else
15151512
{
@@ -1533,6 +1530,137 @@ renametrig(RenameStmt *stmt)
15331530
returnaddress;
15341531
}
15351532

1533+
/*
1534+
* Subroutine for renametrig -- perform the actual work of renaming one
1535+
* trigger on one table.
1536+
*
1537+
* If the trigger has a name different from the expected one, raise a
1538+
* NOTICE about it.
1539+
*/
1540+
staticvoid
1541+
renametrig_internal(Relationtgrel,Relationtargetrel,HeapTupletrigtup,
1542+
constchar*newname,constchar*expected_name)
1543+
{
1544+
HeapTupletuple;
1545+
Form_pg_triggertgform;
1546+
ScanKeyDatakey[2];
1547+
SysScanDesctgscan;
1548+
1549+
/* If the trigger already has the new name, nothing to do. */
1550+
tgform= (Form_pg_trigger)GETSTRUCT(trigtup);
1551+
if (strcmp(NameStr(tgform->tgname),newname)==0)
1552+
return;
1553+
1554+
/*
1555+
* Before actually trying the rename, search for triggers with the same
1556+
* name. The update would fail with an ugly message in that case, and it
1557+
* is better to throw a nicer error.
1558+
*/
1559+
ScanKeyInit(&key[0],
1560+
Anum_pg_trigger_tgrelid,
1561+
BTEqualStrategyNumber,F_OIDEQ,
1562+
ObjectIdGetDatum(RelationGetRelid(targetrel)));
1563+
ScanKeyInit(&key[1],
1564+
Anum_pg_trigger_tgname,
1565+
BTEqualStrategyNumber,F_NAMEEQ,
1566+
PointerGetDatum(newname));
1567+
tgscan=systable_beginscan(tgrel,TriggerRelidNameIndexId, true,
1568+
NULL,2,key);
1569+
if (HeapTupleIsValid(tuple=systable_getnext(tgscan)))
1570+
ereport(ERROR,
1571+
(errcode(ERRCODE_DUPLICATE_OBJECT),
1572+
errmsg("trigger \"%s\" for relation \"%s\" already exists",
1573+
newname,RelationGetRelationName(targetrel))));
1574+
systable_endscan(tgscan);
1575+
1576+
/*
1577+
* The target name is free; update the existing pg_trigger tuple with it.
1578+
*/
1579+
tuple=heap_copytuple(trigtup);/* need a modifiable copy */
1580+
tgform= (Form_pg_trigger)GETSTRUCT(tuple);
1581+
1582+
/*
1583+
* If the trigger has a name different from what we expected, let the user
1584+
* know. (We can proceed anyway, since we must have reached here following
1585+
* a tgparentid link.)
1586+
*/
1587+
if (strcmp(NameStr(tgform->tgname),expected_name)!=0)
1588+
ereport(NOTICE,
1589+
errmsg("renamed trigger \"%s\" on relation \"%s\"",
1590+
NameStr(tgform->tgname),
1591+
RelationGetRelationName(targetrel)));
1592+
1593+
namestrcpy(&tgform->tgname,newname);
1594+
1595+
CatalogTupleUpdate(tgrel,&tuple->t_self,tuple);
1596+
1597+
InvokeObjectPostAlterHook(TriggerRelationId,tgform->oid,0);
1598+
1599+
/*
1600+
* Invalidate relation's relcache entry so that other backends (and this
1601+
* one too!) are sent SI message to make them rebuild relcache entries.
1602+
* (Ideally this should happen automatically...)
1603+
*/
1604+
CacheInvalidateRelcache(targetrel);
1605+
}
1606+
1607+
/*
1608+
* Subroutine for renametrig -- Helper for recursing to partitions when
1609+
* renaming triggers on a partitioned table.
1610+
*/
1611+
staticvoid
1612+
renametrig_partition(Relationtgrel,OidpartitionId,OidparentTriggerOid,
1613+
constchar*newname,constchar*expected_name)
1614+
{
1615+
SysScanDesctgscan;
1616+
ScanKeyDatakey;
1617+
HeapTupletuple;
1618+
intfoundPG_USED_FOR_ASSERTS_ONLY=0;
1619+
1620+
/*
1621+
* Given a relation and the OID of a trigger on parent relation, find the
1622+
* corresponding trigger in the child and rename that trigger to the given
1623+
* name.
1624+
*/
1625+
ScanKeyInit(&key,
1626+
Anum_pg_trigger_tgrelid,
1627+
BTEqualStrategyNumber,F_OIDEQ,
1628+
ObjectIdGetDatum(partitionId));
1629+
tgscan=systable_beginscan(tgrel,TriggerRelidNameIndexId, true,
1630+
NULL,1,&key);
1631+
while (HeapTupleIsValid(tuple=systable_getnext(tgscan)))
1632+
{
1633+
Form_pg_triggertgform= (Form_pg_trigger)GETSTRUCT(tuple);
1634+
RelationpartitionRel;
1635+
1636+
if (tgform->tgparentid!=parentTriggerOid)
1637+
continue;/* not our trigger */
1638+
1639+
Assert(found++ <=0);
1640+
1641+
partitionRel=table_open(partitionId,NoLock);
1642+
1643+
/* Rename the trigger on this partition */
1644+
renametrig_internal(tgrel,partitionRel,tuple,newname,expected_name);
1645+
1646+
/* And if this relation is partitioned, recurse to its partitions */
1647+
if (partitionRel->rd_rel->relkind==RELKIND_PARTITIONED_TABLE)
1648+
{
1649+
PartitionDescpartdesc=RelationGetPartitionDesc(partitionRel,
1650+
true);
1651+
1652+
for (inti=0;i<partdesc->nparts;i++)
1653+
{
1654+
OidpartitionId=partdesc->oids[i];
1655+
1656+
renametrig_partition(tgrel,partitionId,tgform->oid,newname,
1657+
NameStr(tgform->tgname));
1658+
}
1659+
}
1660+
table_close(partitionRel,NoLock);
1661+
}
1662+
systable_endscan(tgscan);
1663+
}
15361664

15371665
/*
15381666
* EnableDisableTrigger()

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

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3410,3 +3410,79 @@ for each statement execute function trigger_function1();
34103410
delete from convslot_test_parent;
34113411
NOTICE: trigger = bdt_trigger, old_table = (111,tutu), (311,tutu)
34123412
drop table convslot_test_child, convslot_test_parent;
3413+
-- Test trigger renaming on partitioned tables
3414+
create table grandparent (id int, primary key (id)) partition by range (id);
3415+
create table middle partition of grandparent for values from (1) to (10)
3416+
partition by range (id);
3417+
create table chi partition of middle for values from (1) to (5);
3418+
create table cho partition of middle for values from (6) to (10);
3419+
create function f () returns trigger as
3420+
$$ begin return new; end; $$
3421+
language plpgsql;
3422+
create trigger a after insert on grandparent
3423+
for each row execute procedure f();
3424+
alter trigger a on grandparent rename to b;
3425+
select tgrelid::regclass, tgname,
3426+
(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname
3427+
from pg_trigger where tgrelid in (select relid from pg_partition_tree('grandparent'))
3428+
order by tgname, tgrelid::regclass::text;
3429+
tgrelid | tgname | parent_tgname
3430+
-------------+--------+---------------
3431+
chi | b | b
3432+
cho | b | b
3433+
grandparent | b |
3434+
middle | b | b
3435+
(4 rows)
3436+
3437+
alter trigger a on only grandparent rename to b;-- ONLY not supported
3438+
ERROR: syntax error at or near "only"
3439+
LINE 1: alter trigger a on only grandparent rename to b;
3440+
^
3441+
alter trigger b on middle rename to c;-- can't rename trigger on partition
3442+
ERROR: cannot rename trigger "b" on table "middle"
3443+
HINT: Rename trigger on partitioned table "grandparent" instead.
3444+
create trigger c after insert on middle
3445+
for each row execute procedure f();
3446+
alter trigger b on grandparent rename to c;
3447+
ERROR: trigger "c" for relation "middle" already exists
3448+
-- Rename cascading does not affect statement triggers
3449+
create trigger p after insert on grandparent for each statement execute function f();
3450+
create trigger p after insert on middle for each statement execute function f();
3451+
alter trigger p on grandparent rename to q;
3452+
select tgrelid::regclass, tgname,
3453+
(select tgname from pg_trigger tr where tr.oid = pg_trigger.tgparentid) parent_tgname
3454+
from pg_trigger where tgrelid in (select relid from pg_partition_tree('grandparent'))
3455+
order by tgname, tgrelid::regclass::text;
3456+
tgrelid | tgname | parent_tgname
3457+
-------------+--------+---------------
3458+
chi | b | b
3459+
cho | b | b
3460+
grandparent | b |
3461+
middle | b | b
3462+
chi | c | c
3463+
cho | c | c
3464+
middle | c |
3465+
middle | p |
3466+
grandparent | q |
3467+
(9 rows)
3468+
3469+
drop table grandparent;
3470+
-- Trigger renaming does not recurse on legacy inheritance
3471+
create table parent (a int);
3472+
create table child () inherits (parent);
3473+
create trigger parenttrig after insert on parent
3474+
for each row execute procedure f();
3475+
create trigger parenttrig after insert on child
3476+
for each row execute procedure f();
3477+
alter trigger parenttrig on parent rename to anothertrig;
3478+
\d+ child
3479+
Table "public.child"
3480+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
3481+
--------+---------+-----------+----------+---------+---------+--------------+-------------
3482+
a | integer | | | | plain | |
3483+
Triggers:
3484+
parenttrig AFTER INSERT ON child FOR EACH ROW EXECUTE FUNCTION f()
3485+
Inherits: parent
3486+
3487+
drop table parent, child;
3488+
drop function f();

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp