Fix another issue with ENABLE/DISABLE TRIGGER on partitioned tables.

In v13 and v14, the ENABLE/DISABLE TRIGGER USER variant malfunctioned
on cloned triggers, failing to find the clones because it thought they
were system triggers.  Other variants of ENABLE/DISABLE TRIGGER would
improperly apply a superuserness check.  Fix by adjusting the is-it-
a-system-trigger check to match reality in those branches.  (As far
as I can find, this is the only place that got it wrong.)

There's no such bug in v15/HEAD, because we revised the catalog
representation of system triggers to be what this code was expecting.
However, add the test case to these branches anyway, because this area
is visibly pretty fragile.  Also remove an obsoleted comment.

The recent v15/HEAD commit 6949b921d fixed a nearby bug.  I now see
that my commit message for that was inaccurate: the behavior of
recursing to clone triggers is older than v15, but it didn't apply
to the case in v13/v14 because in those branches parent partitioned
tables have no pg_trigger entries for foreign-key triggers.  But add
the test case from that commit to v13/v14, just to show what is
happening there.

Per bug #17886 from DzmitryH.

Discussion: https://postgr.es/m/17886-5406d5d828aa4aa3@postgresql.org

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 7326ece5565091835c411a8e7e9b451fbbe702d4..08afc4a4ad5da91f7f8b1ee6a6b817fa315e966a 100644 (file)
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -868,11 +868,6 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
 
    /*
     * Build the new pg_trigger tuple.
-    *
-    * When we're creating a trigger in a partition, we mark it as internal,
-    * even though we don't do the isInternal magic in this function.  This
-    * makes the triggers in partitions identical to the ones in the
-    * partitioned tables, except that they are marked internal.
     */
    memset(nulls, false, sizeof(nulls));
 

diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index 66d473ab03140fe1e0cf1badf379e49a8701b2dc..cc15f5c715c8cc688396d14d9af55fae42908a99 100644 (file)
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -2718,6 +2718,18 @@ select tgrelid::regclass, tgname, tgenabled from pg_trigger
  parent  | tg_stmt | A
 (3 rows)
 
+-- This variant malfunctioned in some releases.
+alter table parent disable trigger user;
+select tgrelid::regclass, tgname, tgenabled from pg_trigger
+  where tgrelid in ('parent'::regclass, 'child1'::regclass)
+  order by tgrelid::regclass::text, tgname;
+ tgrelid | tgname  | tgenabled 
+---------+---------+-----------
+ child1  | tg      | D
+ parent  | tg      | D
+ parent  | tg_stmt | D
+(3 rows)
+
 drop table parent, child1;
 -- Check processing of foreign key triggers
 create table parent (a int primary key, f int references parent)

diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index 4222a8500b4097859ea0da607817b990eb9ff6b5..d29e98d2ac944007d9eb4f3032ce9efb8094fde7 100644 (file)
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -1878,6 +1878,11 @@ select tgrelid::regclass, tgname, tgenabled from pg_trigger
 -- The following is a no-op for the parent trigger but not so
 -- for the child trigger, so recursion should be applied.
 alter table 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, tgname;
+-- This variant malfunctioned in some releases.
+alter table parent disable trigger user;
 select tgrelid::regclass, tgname, tgenabled from pg_trigger
   where tgrelid in ('parent'::regclass, 'child1'::regclass)
   order by tgrelid::regclass::text, tgname;