Skip to content

Commit

Permalink
Fix ALTER TABLE .. ENABLE/DISABLE TRIGGER recursion
Browse files Browse the repository at this point in the history
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/[email protected]
  • Loading branch information
alvherre committed Oct 20, 2020
1 parent 03d51b7 commit bbb927b
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 21 deletions.
2 changes: 2 additions & 0 deletions src/backend/commands/tablecmds.c
Original file line number Diff line number Diff line change
Expand Up @@ -4321,6 +4321,8 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_DisableTrigAll:
case AT_DisableTrigUser:
ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context);
pass = AT_PASS_MISC;
break;
case AT_EnableRule: /* ENABLE/DISABLE RULE variants */
Expand Down
21 changes: 0 additions & 21 deletions src/backend/commands/trigger.c
Original file line number Diff line number Diff line change
Expand Up @@ -1531,27 +1531,6 @@ EnableDisableTrigger(Relation rel, const char *tgname,

heap_freetuple(newtup);

/*
* When altering FOR EACH ROW triggers on a partitioned table, do
* the same on the partitions as well.
*/
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
(TRIGGER_FOR_ROW(oldtrig->tgtype)))
{
PartitionDesc partdesc = RelationGetPartitionDesc(rel);
int i;

for (i = 0; i < partdesc->nparts; i++)
{
Relation part;

part = relation_open(partdesc->oids[i], lockmode);
EnableDisableTrigger(part, NameStr(oldtrig->tgname),
fires_when, skip_system, lockmode);
table_close(part, NoLock); /* keep lock till commit */
}
}

changed = true;
}

Expand Down
56 changes: 56 additions & 0 deletions src/test/regress/expected/triggers.out
Original file line number Diff line number Diff line change
Expand Up @@ -2512,6 +2512,62 @@ select tgrelid::regclass, count(*) from pg_trigger
(5 rows)

drop table trg_clone;
-- Test the interaction between ALTER TABLE .. DISABLE TRIGGER and
-- both kinds of inheritance. Historically, legacy inheritance has
-- not recursed to children, so that behavior is preserved.
create table parent (a int);
create table child1 () inherits (parent);
create function trig_nothing() returns trigger language plpgsql
as $$ begin return null; end $$;
create trigger tg after insert on parent
for each row execute function trig_nothing();
create trigger tg after insert on child1
for each row execute function trig_nothing();
alter table parent disable trigger tg;
select tgrelid::regclass, tgname, tgenabled from pg_trigger
where tgrelid in ('parent'::regclass, 'child1'::regclass)
order by tgrelid::regclass::text;
tgrelid | tgname | tgenabled
---------+--------+-----------
child1 | tg | O
parent | tg | D
(2 rows)

alter table only parent enable always trigger tg;
select tgrelid::regclass, tgname, tgenabled from pg_trigger
where tgrelid in ('parent'::regclass, 'child1'::regclass)
order by tgrelid::regclass::text;
tgrelid | tgname | tgenabled
---------+--------+-----------
child1 | tg | O
parent | tg | A
(2 rows)

drop table parent, child1;
create table parent (a int) partition by list (a);
create table child1 partition of parent for values in (1);
create trigger tg after insert on parent
for each row execute procedure trig_nothing();
select tgrelid::regclass, tgname, tgenabled from pg_trigger
where tgrelid in ('parent'::regclass, 'child1'::regclass)
order by tgrelid::regclass::text;
tgrelid | tgname | tgenabled
---------+--------+-----------
child1 | tg | O
parent | tg | O
(2 rows)

alter table only parent enable always trigger tg;
select tgrelid::regclass, tgname, tgenabled from pg_trigger
where tgrelid in ('parent'::regclass, 'child1'::regclass)
order by tgrelid::regclass::text;
tgrelid | tgname | tgenabled
---------+--------+-----------
child1 | tg | O
parent | tg | A
(2 rows)

drop table parent, child1;
--
-- Test the interaction between transition tables and both kinds of
-- inheritance. We'll dump the contents of the transition tables in a
Expand Down
35 changes: 35 additions & 0 deletions src/test/regress/sql/triggers.sql
Original file line number Diff line number Diff line change
Expand Up @@ -1749,6 +1749,41 @@ select tgrelid::regclass, count(*) from pg_trigger
group by tgrelid::regclass order by tgrelid::regclass;
drop table trg_clone;

-- Test the interaction between ALTER TABLE .. DISABLE TRIGGER and
-- both kinds of inheritance. Historically, legacy inheritance has
-- not recursed to children, so that behavior is preserved.
create table parent (a int);
create table child1 () inherits (parent);
create function trig_nothing() returns trigger language plpgsql
as $$ begin return null; end $$;
create trigger tg after insert on parent
for each row execute function trig_nothing();
create trigger tg after insert on child1
for each row execute function trig_nothing();
alter table parent disable trigger tg;
select tgrelid::regclass, tgname, tgenabled from pg_trigger
where tgrelid in ('parent'::regclass, 'child1'::regclass)
order by tgrelid::regclass::text;
alter table only parent enable always trigger tg;
select tgrelid::regclass, tgname, tgenabled from pg_trigger
where tgrelid in ('parent'::regclass, 'child1'::regclass)
order by tgrelid::regclass::text;
drop table parent, child1;

create table parent (a int) partition by list (a);
create table child1 partition of parent for values in (1);
create trigger tg after insert on parent
for each row execute procedure trig_nothing();
select tgrelid::regclass, tgname, tgenabled from pg_trigger
where tgrelid in ('parent'::regclass, 'child1'::regclass)
order by tgrelid::regclass::text;
alter table only parent enable always trigger tg;
select tgrelid::regclass, tgname, tgenabled from pg_trigger
where tgrelid in ('parent'::regclass, 'child1'::regclass)
order by tgrelid::regclass::text;
drop table parent, child1;


--
-- Test the interaction between transition tables and both kinds of
-- inheritance. We'll dump the contents of the transition tables in a
Expand Down

0 comments on commit bbb927b

Please sign in to comment.