MIME-Version: 1.0
References: <20250331212648.ad4ab804559001d7f0788741@sraoss.co.jp>
 <CAOVWO5q8cfbHiZuL-wSxknnriPCBYpqRCnQKfH38WuLwFuxkSw@mail.gmail.com>
 <20260327005646.8b9df4658b2dd510d3f95ec0@sraoss.co.jp>
In-Reply-To: <20260327005646.8b9df4658b2dd510d3f95ec0@sraoss.co.jp>
From: Amit Langote <amitlangote09@gmail.com>
Date: Fri, 27 Mar 2026 09:39:17 +0900
Message-ID: 
 <CA+HiwqGFuhuSus8L7O36CYoeeZtGJ4PO0cR94qKDbtMKR4T2OA@mail.gmail.com>
Subject: Re: Add comments about fire_triggers argument in ri_triggers.c
To: Yugo Nagata <nagata@sraoss.co.jp>
Cc: surya poondla <suryapoondla4@gmail.com>, pgsql-hackers@postgresql.org,
	Kirill Reshke <reshkekirill@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CA%2BHiwqGFuhuSus8L7O36CYoeeZtGJ4PO0cR94qKDbtMKR4T2OA%40mail.gmail.com>
Precedence: bulk

On Fri, Mar 27, 2026 at 12:56=E2=80=AFAM Yugo Nagata <nagata@sraoss.co.jp> =
wrote:
>
> Hi,
>
> Thank you all for the review and comments.
>
> > Yes Amit, I agree that SPI_execute_snapshot() comments do provide some
> > context on AFTER triggers, but I still feel the newly added comment
> > in ri_PerformCheck() gives additional context on why the fire_triggers =
is
> > set to false.
>
> Yes, that is what I intended. The existing comments on
> SPI_execute_snapshot() explain how the fire_triggers parameter works,
> but I would like to add a comment explaining why the AFTER trigger for
> RI needs to set it to false.
>
> If the explanation of the effect of fire_triggers seems redundant, I am
> fine with the following shorter version:
>
> +        * Set fire_triggers to false to ensure that check triggers fire =
after all
> +        * RI updates on the same row are complete.

Thanks for the updated patch.  Yes, adding the comment might be good,
but I'd suggest a small tweak:

+        * Set fire_triggers to false to ensure that AFTER triggers
are queued in
+        * the outer query's after-trigger context and fire after all
RI updates on
+        * the same row are complete, rather than immediately.

Two changes:

* "check triggers" -> "AFTER triggers", since fire_triggers=3Dfalse
affects any AFTER triggers queued during the SPI execution, not just
RI check triggers.

* mention of the outer query's after-trigger context to explain the
mechanism by which the deferral works.

Does that additional context help?

--=20
Thanks, Amit Langote