Re: FOR EACH ROW triggers, on partitioend tables, with indexes?

Justin Pryzby <pryzby@telsasoft.com>

From: Justin Pryzby <pryzby@telsasoft.com>
To: David Rowley <dgrowleyml@gmail.com>
Cc: pgsql-hackers@postgresql.org, Tomas Vondra <tomas.vondra@postgresql.org>, Peter Smith <smithpb2250@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: 2022-09-01T05:31:46Z
Lists: pgsql-hackers
On Thu, Sep 01, 2022 at 04:19:37PM +1200, David Rowley wrote:
> On Sat, 20 Aug 2022 at 09:18, Justin Pryzby <pryzby@telsasoft.com> wrote:
> > Is it somwhow possible to call CreateTrigger() to create a FOR EACH ROW
> > trigger, with an index, and not internally ?
> 
> I've been looking over this and I very much agree that the code looks
> very broken. As for whether this is dead code or not, I've been
> looking at that too...
> 
> At trigger.c:1147 we have: if (partition_recurse).  partition_recurse
> can only ever be true if isInternal == false per trigger.c:367's
> "partition_recurse = !isInternal && stmt->row &&". isInternal is a
> parameter to the function.  Also, the code in question only triggers
> when the indexOid parameter is a valid oid.  So it should just be a
> matter of looking for usages of CreateTriggerFiringOn() which pass
> isInternal as false and pass a valid indexOid.
> 
> There seems to be no direct calls doing this, but we do also call this
> function via CreateTrigger() and I can see only 1 call to
> CreateTrigger() that passes isInternal as false, but that explicitly
> passes indexOid as InvalidOid, so this code looks very much dead to
> me.
> 
> Alvaro, any objections to just ripping this out? aka, the attached.

It's possible that extensions or 3rd party code or forks use this, no ?
In that case, it might be "not dead" ..

> +		 * that ever changes then we'll need to quite code here to find the

quite?  write?  quire?  acquire?  quine? 

-- 
Justin



Commits

  1. Fix typo in 16d69ec29

  2. Remove buggy and dead code from CreateTriggerFiringOn

  3. Allow FOR EACH ROW triggers on partitioned tables