Fix ALTER TABLE .. ENABLE/DISABLE TRIGGER recursion

More precisely, correctly handle the ONLY flag indicating not to
recurse.  This was implemented in 86f575948c77 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

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 511f015a861cfd87527a02b3e1e596d1b9368528..a29c14bf1cf0a9d9495aae3bccde73201f44b1b7 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -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 */

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 5719672f4ce15f92e536e42bacf7c74fe3519d11..28b98d10ae85b17f698915e7f3f808fb96c82c67 100644 (file)
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -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;
        }
 

diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index 5e76b3a47e7da7b2d7403fbec0c91ed5ddc9592d..57efa290207a2daf580af0a89201a4cd20f107c1 100644 (file)
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -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

diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index e228d0a8a5b6e9f6e3efc67bc4314585a527f4c2..8f66df9f3b41e29e940565f89234866981bf5a67 100644 (file)
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -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