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

On Tue, May 24, 2022 at 3:11 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> Simon reported $subject off-list.
>
> For triggers on partitioned tables, various enable/disable trigger
> variants recurse to also process partitions' triggers by way of
> ATSimpleRecursion() done in the "prep" phase. While that way of
> recursing is fine for row-level triggers which are cloned to
> partitions, it isn't for statement-level triggers which are not
> cloned, so you get an unexpected error as follows:
>
> create table p (a int primary key) partition by list (a);
> create table p1 partition of p for values in (1);
> create function trigfun () returns trigger language plpgsql as $$
> begin raise notice 'insert on p'; end; $$;
> create trigger trig before insert on p for statement execute function trigfun();
> alter table p disable trigger trig;
> ERROR: trigger "trig" for table "p1" does not exist
>
> The problem is that ATPrepCmd() is too soon to perform the recursion
> in this case as it's not known at that stage if the trigger being
> enabled/disabled is row-level or statement level, so it's better to
> perform it during ATExecCmd(). Actually, that is how it used to be
> done before bbb927b4db9b changed things to use ATSimpleRecursion() to
> fix a related problem, which was that the ONLY specification was
> ignored by the earlier implementation. The information of whether
> ONLY is specified in a given command is only directly available in the
> "prep" phase and must be remembered somehow if the recursion must be
> handled in the "exec" phase. The way that's typically done that I see
> in tablecmds.c is to have ATPrepCmd() change the AlterTableCmd.subtype
> to a recursive variant of a given sub-command. For example,
> AT_ValidateConstraint by AT_ValidateConstraintRecurse if ONLY is not
> specified.
>
> So, I think we should do something like the attached. A lot of
> boilerplate is needed given that the various enable/disable trigger
> variants are represented as separate sub-commands (AlterTableCmd
> subtypes), which can perhaps be avoided by inventing a
> EnableDisableTrigStmt sub-command node that stores (only?) the recurse
> flag.

Added to the next CF: https://commitfest.postgresql.org/38/3728/

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