On Tue, May 24, 2022 at 3:11 PM Amit Langote <amitlangot...@gmail.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