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.

I've attached an updated version reflecting this.

Regards,
Yugo Nagata

-- 
Yugo Nagata <[email protected]>
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index d22b8ef7f3c..4cbce56951b 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -2582,7 +2582,12 @@ 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 to ensure that 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,

Reply via email to