MIME-Version: 1.0
References: <20250331212648.ad4ab804559001d7f0788741@sraoss.co.jp>
In-Reply-To: <20250331212648.ad4ab804559001d7f0788741@sraoss.co.jp>
From: Kirill Reshke <reshkekirill@gmail.com>
Date: Mon, 24 Nov 2025 14:22:56 +0500
Message-ID: 
 <CALdSSPg-rnJ27pxR=7Z20q8+Vob2dqjSStUOyWCqpRtNWKw-qQ@mail.gmail.com>
Subject: Re: Add comments about fire_triggers argument in ri_triggers.c
To: Yugo Nagata <nagata@sraoss.co.jp>
Cc: pgsql-hackers@postgresql.org
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CALdSSPg-rnJ27pxR%3D7Z20q8%2BVob2dqjSStUOyWCqpRtNWKw-qQ%40mail.gmail.com>
Precedence: bulk

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