Hi, On Mon, Nov 24, 2025 at 6:23 PM Kirill Reshke <[email protected]> wrote: > On Mon, 31 Mar 2025 at 17:27, Yugo Nagata <[email protected]> wrote: > > > > Hi, > > > > SPI_execute_snapshot() has a argument called "fire_triggers". If this is > > false, > > AFTER triggers are postponed to end of the query. This is true in normal > > case, > > but set to false in RI triggers. > > > > This is introduced by 9cb84097623e in 2007. It is aimed to fire check > > triggers > > after all RI updates on the same row are complete. > > > > However, I cannot find explanation of"why this is required" in the > > codebase. > > Therefore, I've attached a patch add comments in ri_trigger.c for > > explaining why > > fire_triggers is specified to false. > > > > SPI_execute_snapshot() are used in a few places in ri_trigger.c, but I added > > the comments only in ri_PerformCheck() because SPI_execute_snapshot() are > > used > > only for SELECT quereis in other places. Therefore, I wonder fire_triggers > > is > > not needed to be false in these places, but I left them as is. > > I checked your patch and I agree that your comment makes things more clear. > > Your patch LGTM
Hmm, isn’t that already explained in the comment for SPI_execute_snapshot()? /* * SPI_execute_snapshot -- identical to SPI_execute_plan, except that we allow * the caller to specify exactly which snapshots to use, which will be * registered here. Also, the caller may specify that AFTER triggers should be * queued as part of the outer query rather than being fired immediately at the * end of the command. That said, we could perhaps add a one-liner in ri_triggers.c as follows: /* * Finally we can run the query. * See SPI_execute_snapshot() for why fire_triggers = false. */ -- Thanks, Amit Langote
