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

David Rowley <dgrowleyml@gmail.com>

From: David Rowley <dgrowleyml@gmail.com>
To: Justin Pryzby <pryzby@telsasoft.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-01T04:19:37Z
Lists: pgsql-hackers

Attachments

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.
I've left an Assert() in there to ensure we notice if we're ever to
start calling CreateTriggerFiringOn() with isInternal == false with a
valid indexOid.

David

Commits

  1. Fix typo in 16d69ec29

  2. Remove buggy and dead code from CreateTriggerFiringOn

  3. Allow FOR EACH ROW triggers on partitioned tables