Fix ALTER TABLE .. ENABLE/DISABLE TRIGGER recursion

More precisely, correctly handle the ONLY flag indicating not to
recurse.  This was implemented in 86f575948c 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
This commit is contained in:
Alvaro Herrera 2020-10-20 19:22:09 -03:00
parent 03d51b776d
commit bbb927b4db
No known key found for this signature in database
GPG Key ID: 1C20ACB9D5C564AE
4 changed files with 93 additions and 21 deletions

View File

@ -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 */

View File

@ -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;
}

View File

@ -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

View File

@ -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