Re: enable/disable broken for statement triggers on partitioned tables

Hi,

On Fri, Jul 8, 2022 at 3:44 AM Dmitry Koval <d.koval@postgrespro.ru> wrote:
> I've looked through the code and everything looks good.
> But there is one thing I doubt.
> Patch changes result of test:
> ----
>
> create function trig_nothing() returns trigger language plpgsql
>    as $$ begin return null; end $$;
> 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; -- no recursion
> select tgrelid::regclass, tgname, tgenabled from pg_trigger
>    where tgrelid in ('parent'::regclass, 'child1'::regclass)
>    order by tgrelid::regclass::text;
> alter table parent enable always trigger tg; -- recursion
> select tgrelid::regclass, tgname, tgenabled from pg_trigger
>    where tgrelid in ('parent'::regclass, 'child1'::regclass)
>    order by tgrelid::regclass::text;
>
> drop table parent, child1;
> drop function trig_nothing();
>
> ----
> Results of vanilla + patch:
> ----
> CREATE FUNCTION
> CREATE TABLE
> CREATE TABLE
> CREATE TRIGGER
>   tgrelid | tgname | tgenabled
> ---------+--------+-----------
>   child1  | tg     | O
>   parent  | tg     | O
> (2 rows)
>
> ALTER TABLE
>   tgrelid | tgname | tgenabled
> ---------+--------+-----------
>   child1  | tg     | O
>   parent  | tg     | A
> (2 rows)
>
> ALTER TABLE
>   tgrelid | tgname | tgenabled
> ---------+--------+-----------
>   child1  | tg     | O
>   parent  | tg     | A
> (2 rows)
>
> DROP TABLE
> DROP FUNCTION
>
> ----
> Results of vanilla:
> ----
> CREATE FUNCTION
> CREATE TABLE
> CREATE TABLE
> CREATE TRIGGER
>   tgrelid | tgname | tgenabled
> ---------+--------+-----------
>   child1  | tg     | O
>   parent  | tg     | O
> (2 rows)
>
> ALTER TABLE
>   tgrelid | tgname | tgenabled
> ---------+--------+-----------
>   child1  | tg     | O
>   parent  | tg     | A
> (2 rows)
>
> ALTER TABLE
>   tgrelid | tgname | tgenabled
> ---------+--------+-----------
>   child1  | tg     | A
>   parent  | tg     | A
> (2 rows)
>
> DROP TABLE
> DROP FUNCTION
> ----
> The patch doesn't start recursion in case 'tgenabled' flag of parent
> table is not changes.
> Probably vanilla result is more correct.

Thanks for the review and this test case.

I agree that the patch shouldn't have changed that behavior, so I've
fixed the patch so that EnableDisableTrigger() recurses even if the
parent trigger is unchanged.


--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com