[go: up one dir, main page]

Fix another issue with ENABLE/DISABLE TRIGGER on partitioned tables.
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 5 Apr 2023 16:56:30 +0000 (12:56 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 5 Apr 2023 16:56:30 +0000 (12:56 -0400)
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

src/backend/commands/trigger.c
src/test/regress/expected/triggers.out
src/test/regress/sql/triggers.sql

index 6b9cc16603e3c87758d17f75f9a43f562ad42320..3a9b1dda3ff59db6dd83a9ee800c9e5ff703aa40 100644 (file)
@@ -1514,7 +1514,7 @@ EnableDisableTriggerNew(Relation rel, const char *tgname,
    {
        Form_pg_trigger oldtrig = (Form_pg_trigger) GETSTRUCT(tuple);
 
-       if (oldtrig->tgisinternal)
+       if (oldtrig->tgisinternal && !OidIsValid(oldtrig->tgparentid))
        {
            /* system trigger ... ok to process? */
            if (skip_system)
index 9766c0f325d23d26580d8d9944ef47e955f2d5d1..b559479f470efb7ff75ddaefe0e543b0275af428 100644 (file)
@@ -2692,6 +2692,45 @@ 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)
+  partition by list (a);
+create table child1 partition of parent for values in (1);
+select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
+  tgfoid::regproc, tgenabled
+  from pg_trigger where tgrelid in ('parent'::regclass, 'child1'::regclass)
+  order by tgrelid::regclass::text, tgfoid;
+ tgrelid |         tgname          |       tgfoid        | tgenabled 
+---------+-------------------------+---------------------+-----------
+ child1  | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | O
+ child1  | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | O
+(2 rows)
+
+-- Before v15, this has no effect because parent has no triggers:
+alter table parent disable trigger all;
+select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
+  tgfoid::regproc, tgenabled
+  from pg_trigger where tgrelid in ('parent'::regclass, 'child1'::regclass)
+  order by tgrelid::regclass::text, tgfoid;
+ tgrelid |         tgname          |       tgfoid        | tgenabled 
+---------+-------------------------+---------------------+-----------
+ child1  | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | O
+ child1  | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | O
+(2 rows)
+
 drop table parent, child1;
 -- Verify that firing state propagates correctly on creation, too
 CREATE TABLE trgfire (i int) PARTITION BY RANGE (i);
index 9e9ce9551f7112ed9bf1d27e77cf57a4dc4a9447..b18faf0d5a2937ad4430d72170de1fe93abbc6c0 100644 (file)
@@ -1848,6 +1848,27 @@ 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;
+drop table parent, child1;
+
+-- Check processing of foreign key triggers
+create table parent (a int primary key, f int references parent)
+  partition by list (a);
+create table child1 partition of parent for values in (1);
+select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
+  tgfoid::regproc, tgenabled
+  from pg_trigger where tgrelid in ('parent'::regclass, 'child1'::regclass)
+  order by tgrelid::regclass::text, tgfoid;
+-- Before v15, this has no effect because parent has no triggers:
+alter table parent disable trigger all;
+select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
+  tgfoid::regproc, tgenabled
+  from pg_trigger where tgrelid in ('parent'::regclass, 'child1'::regclass)
+  order by tgrelid::regclass::text, tgfoid;
 drop table parent, child1;
 
 -- Verify that firing state propagates correctly on creation, too