public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Langote <[email protected]>
To: Kirill Reshke <[email protected]>
Cc: Yugo Nagata <[email protected]>
Cc: [email protected]
Subject: Re: Add comments about fire_triggers argument in ri_triggers.c
Date: Wed, 26 Nov 2025 11:48:06 +0900
Message-ID: <CA+HiwqFg2RTK-VcBLCC5h8K34kNXmQAvz+qB7Wm5cENXw=utWg@mail.gmail.com> (raw)
In-Reply-To: <CALdSSPg-rnJ27pxR=7Z20q8+Vob2dqjSStUOyWCqpRtNWKw-qQ@mail.gmail.com>
References: <[email protected]>
<CALdSSPg-rnJ27pxR=7Z20q8+Vob2dqjSStUOyWCqpRtNWKw-qQ@mail.gmail.com>
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
view thread (7+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: Add comments about fire_triggers argument in ri_triggers.c
In-Reply-To: <CA+HiwqFg2RTK-VcBLCC5h8K34kNXmQAvz+qB7Wm5cENXw=utWg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox