Add comments about fire_triggers argument in ri_triggers.c
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>
Attachments:
add_comments_fire_triggers_in_ri_triggers.patchtext/x-diff; name=add_comments_fire_triggers_in_ri_triggers.patchDownload
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index c4ff18ce65e..45d7e6268e4 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -2580,7 +2580,13 @@ ri_PerformCheck(const RI_ConstraintInfo *riinfo,
save_sec_context | SECURITY_LOCAL_USERID_CHANGE |
SECURITY_NOFORCE_RLS);
- /* Finally we can run the query. */
+ /*
+ * Finally we can run the query.
+ *
+ * Set fire_triggers to false so that AFTER triggers run at the end of
+ * the query. This ensures check triggers fire after all RI updates on
+ * the same row are complete.
+ */
spi_result = SPI_execute_snapshot(qplan,
vals, nulls,
test_snapshot, crosscheck_snapshot,
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
Hi,
On Mon, Nov 24, 2025 at 6:23 PM Kirill Reshke <reshkekirill@gmail.com> wrote:
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.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