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
- remove_dead_code_from_CreateTriggerFiringOn.patch (text/plain) patch
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
-
Fix typo in 16d69ec29
- c89b44a68d91 16.0 landed
-
Remove buggy and dead code from CreateTriggerFiringOn
- 16d69ec29b11 16.0 landed
-
Allow FOR EACH ROW triggers on partitioned tables
- 86f575948c77 11.0 cited