Re: Add comments about fire_triggers argument in ri_triggers.c

On Mon, 31 Mar 2025 at 17:27, Yugo Nagata <nagata@sraoss.co.jp> 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.
>
> Regards,
> Yugo Nagata
>
> --
> Yugo Nagata <nagata@sraoss.co.jp>

Hi!

I checked your patch and I agree that your comment makes things more clear.

Your patch LGTM

-- 
Best regards,
Kirill Reshke