enable/disable broken for statement triggers on partitioned tables - Mailing list pgsql-hackers

From Amit Langote
Subject enable/disable broken for statement triggers on partitioned tables
Date
Msg-id CA+HiwqG-cZT3XzGAnEgZQLoQbyfJApVwOTQaCaas1mhpf+4V5A@mail.gmail.com
Whole thread Raw
Responses Re: enable/disable broken for statement triggers on partitioned tables  (Amit Langote <amitlangote09@gmail.com>)
Re: enable/disable broken for statement triggers on partitioned tables  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
List pgsql-hackers
Hi,

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.

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

Attachment

pgsql-hackers by date:

Previous
From: "bucoo"
Date:
Subject: re: partition wise aggregate wrong rows cost
Next
From: Nikolay Shaplov
Date:
Subject: Re: Zstandard support for toast compression