Skip to content

Commit a795f67

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 6e03a8a commit a795f67

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
@@ -3978,6 +3978,8 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
39783978
case AT_DisableTrigAll:
39793979
case AT_DisableTrigUser:
39803980
ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
3981+
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
3982+
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
39813983
pass = AT_PASS_MISC;
39823984
break;
39833985
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
@@ -1847,27 +1847,6 @@ EnableDisableTrigger(Relation rel, const char *tgname,
18471847

18481848
heap_freetuple(newtup);
18491849

1850-
/*
1851-
* When altering FOR EACH ROW triggers on a partitioned table, do
1852-
* the same on the partitions as well.
1853-
*/
1854-
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
1855-
(TRIGGER_FOR_ROW(oldtrig->tgtype)))
1856-
{
1857-
PartitionDesc partdesc = RelationGetPartitionDesc(rel);
1858-
int i;
1859-
1860-
for (i = 0; i < partdesc->nparts; i++)
1861-
{
1862-
Relation part;
1863-
1864-
part = relation_open(partdesc->oids[i], lockmode);
1865-
EnableDisableTrigger(part, NameStr(oldtrig->tgname),
1866-
fires_when, skip_system, lockmode);
1867-
heap_close(part, NoLock); /* keep lock till commit */
1868-
}
1869-
}
1870-
18711850
changed = true;
18721851
}
18731852

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

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

24422442
drop table trg_clone;
2443+
-- Test the interaction between ALTER TABLE .. DISABLE TRIGGER and
2444+
-- both kinds of inheritance. Historically, legacy inheritance has
2445+
-- not recursed to children, so that behavior is preserved.
2446+
create table parent (a int);
2447+
create table child1 () inherits (parent);
2448+
create function trig_nothing() returns trigger language plpgsql
2449+
as $$ begin return null; end $$;
2450+
create trigger tg after insert on parent
2451+
for each row execute function trig_nothing();
2452+
create trigger tg after insert on child1
2453+
for each row execute function trig_nothing();
2454+
alter table parent disable trigger tg;
2455+
select tgrelid::regclass, tgname, tgenabled from pg_trigger
2456+
where tgrelid in ('parent'::regclass, 'child1'::regclass)
2457+
order by tgrelid::regclass::text;
2458+
tgrelid | tgname | tgenabled
2459+
---------+--------+-----------
2460+
child1 | tg | O
2461+
parent | tg | D
2462+
(2 rows)
2463+
2464+
alter table only parent enable always trigger tg;
2465+
select tgrelid::regclass, tgname, tgenabled from pg_trigger
2466+
where tgrelid in ('parent'::regclass, 'child1'::regclass)
2467+
order by tgrelid::regclass::text;
2468+
tgrelid | tgname | tgenabled
2469+
---------+--------+-----------
2470+
child1 | tg | O
2471+
parent | tg | A
2472+
(2 rows)
2473+
2474+
drop table parent, child1;
2475+
create table parent (a int) partition by list (a);
2476+
create table child1 partition of parent for values in (1);
2477+
create trigger tg after insert on parent
2478+
for each row execute procedure trig_nothing();
2479+
select tgrelid::regclass, tgname, tgenabled from pg_trigger
2480+
where tgrelid in ('parent'::regclass, 'child1'::regclass)
2481+
order by tgrelid::regclass::text;
2482+
tgrelid | tgname | tgenabled
2483+
---------+--------+-----------
2484+
child1 | tg | O
2485+
parent | tg | O
2486+
(2 rows)
2487+
2488+
alter table only parent enable always trigger tg;
2489+
select tgrelid::regclass, tgname, tgenabled from pg_trigger
2490+
where tgrelid in ('parent'::regclass, 'child1'::regclass)
2491+
order by tgrelid::regclass::text;
2492+
tgrelid | tgname | tgenabled
2493+
---------+--------+-----------
2494+
child1 | tg | O
2495+
parent | tg | A
2496+
(2 rows)
2497+
2498+
drop table parent, child1;
24432499
--
24442500
-- Test the interaction between transition tables and both kinds of
24452501
-- 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
@@ -1703,6 +1703,41 @@ select tgrelid::regclass, count(*) from pg_trigger
17031703
group by tgrelid::regclass order by tgrelid::regclass;
17041704
drop table trg_clone;
17051705

1706+
-- Test the interaction between ALTER TABLE .. DISABLE TRIGGER and
1707+
-- both kinds of inheritance. Historically, legacy inheritance has
1708+
-- not recursed to children, so that behavior is preserved.
1709+
create table parent (a int);
1710+
create table child1 () inherits (parent);
1711+
create function trig_nothing() returns trigger language plpgsql
1712+
as $$ begin return null; end $$;
1713+
create trigger tg after insert on parent
1714+
for each row execute function trig_nothing();
1715+
create trigger tg after insert on child1
1716+
for each row execute function trig_nothing();
1717+
alter table parent disable trigger tg;
1718+
select tgrelid::regclass, tgname, tgenabled from pg_trigger
1719+
where tgrelid in ('parent'::regclass, 'child1'::regclass)
1720+
order by tgrelid::regclass::text;
1721+
alter table only parent enable always trigger tg;
1722+
select tgrelid::regclass, tgname, tgenabled from pg_trigger
1723+
where tgrelid in ('parent'::regclass, 'child1'::regclass)
1724+
order by tgrelid::regclass::text;
1725+
drop table parent, child1;
1726+
1727+
create table parent (a int) partition by list (a);
1728+
create table child1 partition of parent for values in (1);
1729+
create trigger tg after insert on parent
1730+
for each row execute procedure trig_nothing();
1731+
select tgrelid::regclass, tgname, tgenabled from pg_trigger
1732+
where tgrelid in ('parent'::regclass, 'child1'::regclass)
1733+
order by tgrelid::regclass::text;
1734+
alter table only parent enable always trigger tg;
1735+
select tgrelid::regclass, tgname, tgenabled from pg_trigger
1736+
where tgrelid in ('parent'::regclass, 'child1'::regclass)
1737+
order by tgrelid::regclass::text;
1738+
drop table parent, child1;
1739+
1740+
17061741
--
17071742
-- Test the interaction between transition tables and both kinds of
17081743
-- inheritance. We'll dump the contents of the transition tables in a

0 commit comments

Comments
 (0)