Re: BUG #17886: Error disabling user triggers on a partitioned table

On 2023-Apr-04, Tom Lane wrote:

> PG Bug reporting form <noreply@postgresql.org> writes:
> > result:
> > postgres=# alter table public.test DISABLE TRIGGER USER;
> > ERROR:  42704: trigger "trigger_test" for table "test_def" does not exist
> > LOCATION:  EnableDisableTriggerNew, trigger.c:1658
> 
> > Expected Result (Postgresql 15.2 and 14.4 - fine):
> > testdb=> alter table public.test DISABLE TRIGGER USER;
> > ALTER TABLE
> 
> Commit ec0925c22 seems to have been quite snakebit.

No kidding :-(  Clearly, commit 86f575948c77 ("Allow FOR EACH ROW
triggers on partitioned tables") should have included more tests.

> I already fixed
> a deficiency in it in v15/HEAD, but here we have a different symptom
> in the older branches.  What's happening is that EnableDisableTrigger
> is ignoring the child trigger because it has tgisinternal set to true
> and the command passes skip_system = true.

Hmm, right.

> I'm inclined to think that in the older branches (pre f4566345c)
> we need to do
> 
> -       if (oldtrig->tgisinternal)
> +       if (oldtrig->tgisinternal && !OidIsValid(oldtrig->tgparentid))
>         {
>             /* system trigger ... ok to process? */
> 
> but I've not tested that.  Alvaro, what do you think?

Oh, that's a nice and clean solution.  I tested it (on 13 and 14) and it
does solve the problem, and no regression tests fail, but I didn't try
to break it further.  12 and back are unaffected, for lack of
86f575948c77.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Nadie está tan esclavizado como el que se cree libre no siéndolo" (Goethe)