Skip to content

Commit bbb927b

Browse files
alvherrealvherre
committed
Fix ALTER TABLE .. ENABLE/DISABLE TRIGGER recursion
More precisely, correctly handle the ONLY flag indicating not to recurse. This was implemented in 86f5759 by recursing in trigger.c, but that's the wrong place; use ATSimpleRecursion instead, which behaves properly. However, because legacy inheritance has never recursed in that situation, make sure to do that only for new-style partitioning. I noticed this problem while testing a fix for another bug in the vicinity. This has been wrong all along, so backpatch to 11. Discussion: https://postgr.es/m/20201016235925.GA29829@alvherre.pgsql
1 parent 03d51b7 commit bbb927b

File tree

4 files changed

+93
-21
lines changed

4 files changed

+93
-21
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4321,6 +4321,8 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
43214321
case AT_DisableTrigAll:
43224322
case AT_DisableTrigUser:
43234323
ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
4324+
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
4325+
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context);
43244326
pass = AT_PASS_MISC;
43254327
break;
43264328
case AT_EnableRule: /* ENABLE/DISABLE RULE variants */

‎src/backend/commands/trigger.c

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1531,27 +1531,6 @@ EnableDisableTrigger(Relation rel, const char *tgname,
15311531

15321532
heap_freetuple(newtup);
15331533

1534-
/*
1535-
* When altering FOR EACH ROW triggers on a partitioned table, do
1536-
* the same on the partitions as well.
1537-
*/
1538-
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
1539-
(TRIGGER_FOR_ROW(oldtrig->tgtype)))
1540-
{
1541-
PartitionDesc partdesc = RelationGetPartitionDesc(rel);
1542-
int i;
1543-
1544-
for (i = 0; i < partdesc->nparts; i++)
1545-
{
1546-
Relation part;
1547-
1548-
part = relation_open(partdesc->oids[i], lockmode);
1549-
EnableDisableTrigger(part, NameStr(oldtrig->tgname),
1550-
fires_when, skip_system, lockmode);
1551-
table_close(part, NoLock); /* keep lock till commit */
1552-
}
1553-
}
1554-
15551534
changed = true;
15561535
}
15571536

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2512,6 +2512,62 @@ select tgrelid::regclass, count(*) from pg_trigger
25122512
(5 rows)
25132513

25142514
drop table trg_clone;
2515+
-- Test the interaction between ALTER TABLE .. DISABLE TRIGGER and
2516+
-- both kinds of inheritance. Historically, legacy inheritance has
2517+
-- not recursed to children, so that behavior is preserved.
2518+
create table parent (a int);
2519+
create table child1 () inherits (parent);
2520+
create function trig_nothing() returns trigger language plpgsql
2521+
as $$ begin return null; end $$;
2522+
create trigger tg after insert on parent
2523+
for each row execute function trig_nothing();
2524+
create trigger tg after insert on child1
2525+
for each row execute function trig_nothing();
2526+
alter table parent disable trigger tg;
2527+
select tgrelid::regclass, tgname, tgenabled from pg_trigger
2528+
where tgrelid in ('parent'::regclass, 'child1'::regclass)
2529+
order by tgrelid::regclass::text;
2530+
tgrelid | tgname | tgenabled
2531+
---------+--------+-----------
2532+
child1 | tg | O
2533+
parent | tg | D
2534+
(2 rows)
2535+
2536+
alter table only parent enable always trigger tg;
2537+
select tgrelid::regclass, tgname, tgenabled from pg_trigger
2538+
where tgrelid in ('parent'::regclass, 'child1'::regclass)
2539+
order by tgrelid::regclass::text;
2540+
tgrelid | tgname | tgenabled
2541+
---------+--------+-----------
2542+
child1 | tg | O
2543+
parent | tg | A
2544+
(2 rows)
2545+
2546+
drop table parent, child1;
2547+
create table parent (a int) partition by list (a);
2548+
create table child1 partition of parent for values in (1);
2549+
create trigger tg after insert on parent
2550+
for each row execute procedure trig_nothing();
2551+
select tgrelid::regclass, tgname, tgenabled from pg_trigger
2552+
where tgrelid in ('parent'::regclass, 'child1'::regclass)
2553+
order by tgrelid::regclass::text;
2554+
tgrelid | tgname | tgenabled
2555+
---------+--------+-----------
2556+
child1 | tg | O
2557+
parent | tg | O
2558+
(2 rows)
2559+
2560+
alter table only parent enable always trigger tg;
2561+
select tgrelid::regclass, tgname, tgenabled from pg_trigger
2562+
where tgrelid in ('parent'::regclass, 'child1'::regclass)
2563+
order by tgrelid::regclass::text;
2564+
tgrelid | tgname | tgenabled
2565+
---------+--------+-----------
2566+
child1 | tg | O
2567+
parent | tg | A
2568+
(2 rows)
2569+
2570+
drop table parent, child1;
25152571
--
25162572
-- Test the interaction between transition tables and both kinds of
25172573
-- inheritance. We'll dump the contents of the transition tables in a

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1749,6 +1749,41 @@ select tgrelid::regclass, count(*) from pg_trigger
17491749
group by tgrelid::regclass order by tgrelid::regclass;
17501750
drop table trg_clone;
17511751

1752+
-- Test the interaction between ALTER TABLE .. DISABLE TRIGGER and
1753+
-- both kinds of inheritance. Historically, legacy inheritance has
1754+
-- not recursed to children, so that behavior is preserved.
1755+
create table parent (a int);
1756+
create table child1 () inherits (parent);
1757+
create function trig_nothing() returns trigger language plpgsql
1758+
as $$ begin return null; end $$;
1759+
create trigger tg after insert on parent
1760+
for each row execute function trig_nothing();
1761+
create trigger tg after insert on child1
1762+
for each row execute function trig_nothing();
1763+
alter table parent disable trigger tg;
1764+
select tgrelid::regclass, tgname, tgenabled from pg_trigger
1765+
where tgrelid in ('parent'::regclass, 'child1'::regclass)
1766+
order by tgrelid::regclass::text;
1767+
alter table only parent enable always trigger tg;
1768+
select tgrelid::regclass, tgname, tgenabled from pg_trigger
1769+
where tgrelid in ('parent'::regclass, 'child1'::regclass)
1770+
order by tgrelid::regclass::text;
1771+
drop table parent, child1;
1772+
1773+
create table parent (a int) partition by list (a);
1774+
create table child1 partition of parent for values in (1);
1775+
create trigger tg after insert on parent
1776+
for each row execute procedure trig_nothing();
1777+
select tgrelid::regclass, tgname, tgenabled from pg_trigger
1778+
where tgrelid in ('parent'::regclass, 'child1'::regclass)
1779+
order by tgrelid::regclass::text;
1780+
alter table only parent enable always trigger tg;
1781+
select tgrelid::regclass, tgname, tgenabled from pg_trigger
1782+
where tgrelid in ('parent'::regclass, 'child1'::regclass)
1783+
order by tgrelid::regclass::text;
1784+
drop table parent, child1;
1785+
1786+
17521787
--
17531788
-- Test the interaction between transition tables and both kinds of
17541789
-- inheritance. We'll dump the contents of the transition tables in a

0 commit comments

Comments
 (0)