Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 0e6b6f8

Browse files
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 bd0677b commit 0e6b6f8

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
@@ -4237,6 +4237,8 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
42374237
case AT_DisableTrigAll:
42384238
case AT_DisableTrigUser:
42394239
ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
4240+
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
4241+
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
42404242
pass = AT_PASS_MISC;
42414243
break;
42424244
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
@@ -1868,27 +1868,6 @@ EnableDisableTrigger(Relation rel, const char *tgname,
18681868

18691869
heap_freetuple(newtup);
18701870

1871-
/*
1872-
* When altering FOR EACH ROW triggers on a partitioned table, do
1873-
* the same on the partitions as well.
1874-
*/
1875-
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
1876-
(TRIGGER_FOR_ROW(oldtrig->tgtype)))
1877-
{
1878-
PartitionDesc partdesc = RelationGetPartitionDesc(rel);
1879-
int i;
1880-
1881-
for (i = 0; i < partdesc->nparts; i++)
1882-
{
1883-
Relation part;
1884-
1885-
part = relation_open(partdesc->oids[i], lockmode);
1886-
EnableDisableTrigger(part, NameStr(oldtrig->tgname),
1887-
fires_when, skip_system, lockmode);
1888-
table_close(part, NoLock); /* keep lock till commit */
1889-
}
1890-
}
1891-
18921871
changed = true;
18931872
}
18941873

src/test/regress/expected/triggers.out

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

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

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

0 commit comments

Comments
 (0)