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