From 0e8c89928fee4c13f9d698adc9c7ae1b7b66a4fb Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Fri, 4 Sep 2026 20:34:18 +0900
Subject: [PATCH v3 1/2] Don't let ALTER TABLE validation join a trigger's RI
 batch

RI fast-path batching defers a foreign key probe to the end of the
trigger-firing cycle that queued the check.  RI_FKey_check() decided
whether it could batch by asking AfterTriggerIsActive(), that is,
whether any firing cycle was in progress at all.

validateForeignKeyConstraint() calls the insert-check function directly
for each existing row, and no cycle queues those checks.
AfterTriggerIsActive() was taken to distinguish that case, on the
assumption that validation runs outside any firing cycle.  ALTER TABLE
... ADD FOREIGN KEY can itself run from an AFTER trigger, and then it
does not: a cycle is in progress, one that has nothing to do with
validation, and the validation rows joined its batch.  ALTER TABLE
marked the constraint validated as soon as the scan finished, before
anything flushed them.  If the trigger had opened a subtransaction, as a
PL/pgSQL BEGIN ... EXCEPTION block does, its commit discarded the batch
outright, leaving a validated constraint over a table still containing
an orphan row.  Assert-enabled builds crashed instead.

Ambient after-trigger state cannot answer the question: validation and a
genuine RI trigger reach RI_FKey_check() with identical after-trigger
context.  Only the caller knows whether a flush is guaranteed to run
before the result is observed.  Pass it in instead -- give
RI_FKey_check() an allow_batch argument and add RI_FKey_check_validate()
for the validation scan to call -- and remove AfterTriggerIsActive(),
which has no remaining callers.

Reported-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/CAJTYsWU-cihumKaCtnCuV=iuRVcha=3vHa9eXA_vA=cgD2w+Ew@mail.gmail.com
---
 src/backend/commands/tablecmds.c          | 16 ++-----
 src/backend/commands/trigger.c            | 19 ++-------
 src/backend/utils/adt/ri_triggers.c       | 25 +++++++++--
 src/include/commands/trigger.h            |  2 +-
 src/test/regress/expected/foreign_key.out | 52 +++++++++++++++++++++++
 src/test/regress/sql/foreign_key.sql      | 44 +++++++++++++++++++
 6 files changed, 124 insertions(+), 34 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index fd144d783d9..ce866274001 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -14218,8 +14218,8 @@ validateForeignKeyConstraint(char *conname,
 		return;
 
 	/*
-	 * Scan through each tuple, calling RI_FKey_check_ins (insert trigger) as
-	 * if that tuple had just been inserted.  If any of those fail, it should
+	 * Scan through each tuple, calling RI_FKey_check_validate to check that
+	 * it satisfies the constraint.  If any of those checks fail, it should
 	 * ereport(ERROR) and that's that.
 	 */
 	snapshot = RegisterSnapshot(GetLatestSnapshot());
@@ -14233,18 +14233,10 @@ validateForeignKeyConstraint(char *conname,
 
 	while (table_scan_getnextslot(scan, ForwardScanDirection, slot))
 	{
-		LOCAL_FCINFO(fcinfo, 0);
 		TriggerData trigdata = {0};
 
 		CHECK_FOR_INTERRUPTS();
 
-		/*
-		 * Make a call to the trigger function
-		 *
-		 * No parameters are passed, but we do set a context
-		 */
-		MemSet(fcinfo, 0, SizeForFunctionCallInfo(0));
-
 		/*
 		 * We assume RI_FKey_check_ins won't look at flinfo...
 		 */
@@ -14255,9 +14247,7 @@ validateForeignKeyConstraint(char *conname,
 		trigdata.tg_trigslot = slot;
 		trigdata.tg_trigger = &trig;
 
-		fcinfo->context = (Node *) &trigdata;
-
-		RI_FKey_check_ins(fcinfo);
+		RI_FKey_check_validate(&trigdata);
 
 		MemoryContextReset(perTupCxt);
 	}
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 603d798f320..8d24852a8f8 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -3940,8 +3940,9 @@ typedef struct AfterTriggersData
 
 	/*
 	 * Incremented around the trigger-firing loops in AfterTriggerEndQuery,
-	 * AfterTriggerFireDeferred, and AfterTriggerSetState.  Used by
-	 * AfterTriggerIsActive() to signal that after-trigger firing is active.
+	 * AfterTriggerFireDeferred, and AfterTriggerSetState.  Identifies the
+	 * firing cycle a batch callback is registered from; see
+	 * AfterTriggerCurrentFiringDepth().
 	 */
 	int			firing_depth;
 } AfterTriggersData;
@@ -6963,20 +6964,6 @@ FireAfterTriggerBatchCallbacks(List *callbacks)
 	afterTriggers.firing_batch_callbacks = false;
 }
 
-/*
- * AfterTriggerIsActive
- *		Returns true if we're inside the after-trigger framework where
- *		registered batch callbacks will actually be invoked.
- *
- * This is false during validateForeignKeyConstraint(), which calls
- * RI trigger functions directly outside the after-trigger framework.
- */
-bool
-AfterTriggerIsActive(void)
-{
-	return afterTriggers.firing_depth > 0;
-}
-
 /*
  * AfterTriggerCurrentQueryDepth
  *		Return the current after-trigger query nesting depth.
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index d4545618634..1e69e7d93d2 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -412,7 +412,7 @@ static void ri_FastPathTeardown(int depth);
  * Check foreign key existence (combined for INSERT and UPDATE).
  */
 static Datum
-RI_FKey_check(TriggerData *trigdata)
+RI_FKey_check(TriggerData *trigdata, bool allow_batch)
 {
 	RI_ConstraintInfo *riinfo;
 	Relation	fk_rel;
@@ -520,7 +520,7 @@ RI_FKey_check(TriggerData *trigdata)
 	 */
 	if (ri_fastpath_is_applicable(riinfo))
 	{
-		if (AfterTriggerIsActive() && !ri_fastpath_flushing)
+		if (allow_batch && !ri_fastpath_flushing)
 		{
 			/* Batched path: buffer and probe in groups */
 			ri_FastPathBatchAdd(riinfo, fk_rel, newslot);
@@ -675,7 +675,7 @@ RI_FKey_check_ins(PG_FUNCTION_ARGS)
 	ri_CheckTrigger(fcinfo, "RI_FKey_check_ins", RI_TRIGTYPE_INSERT);
 
 	/* Share code with UPDATE case. */
-	return RI_FKey_check((TriggerData *) fcinfo->context);
+	return RI_FKey_check((TriggerData *) fcinfo->context, true);
 }
 
 
@@ -691,7 +691,7 @@ RI_FKey_check_upd(PG_FUNCTION_ARGS)
 	ri_CheckTrigger(fcinfo, "RI_FKey_check_upd", RI_TRIGTYPE_UPDATE);
 
 	/* Share code with INSERT case. */
-	return RI_FKey_check((TriggerData *) fcinfo->context);
+	return RI_FKey_check((TriggerData *) fcinfo->context, true);
 }
 
 
@@ -2001,6 +2001,23 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
 	return true;
 }
 
+/*
+ * RI_FKey_check_validate -
+ *
+ * Check one existing row during ALTER TABLE ... ADD FOREIGN KEY validation.
+ *
+ * This is the check the insert trigger performs, but never batched.  Batching
+ * defers the probe to the end of the firing cycle that queued the check;
+ * validation is queued by no cycle, and ALTER TABLE marks the constraint
+ * validated as soon as the scan completes, so a deferred probe would be
+ * reported -- or discarded -- after the result is already visible.
+ */
+void
+RI_FKey_check_validate(TriggerData *trigdata)
+{
+	(void) RI_FKey_check(trigdata, false);
+}
+
 /*
  * RI_PartitionRemove_Check -
  *
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
index fecdb785f35..6967653ad5c 100644
--- a/src/include/commands/trigger.h
+++ b/src/include/commands/trigger.h
@@ -279,6 +279,7 @@ extern bool RI_FKey_fk_upd_check_required(Trigger *trigger, Relation fk_rel,
 										  TupleTableSlot *oldslot, TupleTableSlot *newslot);
 extern bool RI_Initial_Check(Trigger *trigger,
 							 Relation fk_rel, Relation pk_rel);
+extern void RI_FKey_check_validate(TriggerData *trigdata);
 extern void RI_PartitionRemove_Check(Trigger *trigger, Relation fk_rel,
 									 Relation pk_rel);
 
@@ -308,7 +309,6 @@ typedef void (*AfterTriggerBatchCallback) (void *arg);
 
 extern void RegisterAfterTriggerBatchCallback(AfterTriggerBatchCallback callback,
 											  void *arg);
-extern bool AfterTriggerIsActive(void);
 extern int	AfterTriggerCurrentQueryDepth(void);
 
 extern void AtEOXact_RI(bool isCommit);
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 333994e6505..f1ec4dbd75a 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -4113,3 +4113,55 @@ SELECT count(*) AS deferred_rows FROM fp_deferred_fk;  -- 1, check passed at com
 (1 row)
 
 DROP TABLE fp_deferred_fk, fp_deferred_pk;
+-- ALTER TABLE ... ADD FOREIGN KEY run from inside an AFTER trigger.  The
+-- validation scan must check each row itself rather than batching, so the
+-- orphan row is caught, the ALTER fails, and the constraint is never created.
+--
+-- Row-level security on the referenced table forces the row-at-a-time path;
+-- RI_Initial_Check() cannot use its single query when RLS applies.
+CREATE ROLE regress_fp_alter_role;
+CREATE TABLE fp_alter_pk (id int PRIMARY KEY);
+INSERT INTO fp_alter_pk VALUES (1);
+ALTER TABLE fp_alter_pk ENABLE ROW LEVEL SECURITY;
+CREATE POLICY fp_alter_pk_all ON fp_alter_pk USING (true);
+GRANT REFERENCES, SELECT ON fp_alter_pk TO regress_fp_alter_role;
+CREATE TABLE fp_alter_fk (a int);
+INSERT INTO fp_alter_fk VALUES (1), (999);
+ALTER TABLE fp_alter_fk OWNER TO regress_fp_alter_role;
+CREATE TABLE fp_alter_outer (a int);
+ALTER TABLE fp_alter_outer OWNER TO regress_fp_alter_role;
+CREATE FUNCTION fp_alter_from_trigger() RETURNS trigger
+LANGUAGE plpgsql AS $$
+BEGIN
+    BEGIN
+        EXECUTE 'ALTER TABLE fp_alter_fk ADD CONSTRAINT fp_alter_bad_fk '
+                'FOREIGN KEY (a) REFERENCES fp_alter_pk(id)';
+    EXCEPTION WHEN others THEN
+        RAISE;
+    END;
+    RETURN NEW;
+END$$;
+CREATE TRIGGER fp_alter_trg AFTER INSERT ON fp_alter_outer
+    FOR EACH ROW EXECUTE FUNCTION fp_alter_from_trigger();
+SET ROLE regress_fp_alter_role;
+INSERT INTO fp_alter_outer VALUES (1);
+ERROR:  insert or update on table "fp_alter_fk" violates foreign key constraint "fp_alter_bad_fk"
+DETAIL:  Key (a)=(999) is not present in table "fp_alter_pk".
+CONTEXT:  SQL statement "ALTER TABLE fp_alter_fk ADD CONSTRAINT fp_alter_bad_fk FOREIGN KEY (a) REFERENCES fp_alter_pk(id)"
+PL/pgSQL function fp_alter_from_trigger() line 4 at EXECUTE
+RESET ROLE;
+SELECT conname, convalidated FROM pg_constraint WHERE conname = 'fp_alter_bad_fk';
+ conname | convalidated 
+---------+--------------
+(0 rows)
+
+TABLE fp_alter_fk;
+  a  
+-----
+   1
+ 999
+(2 rows)
+
+DROP TABLE fp_alter_outer, fp_alter_fk, fp_alter_pk;
+DROP FUNCTION fp_alter_from_trigger();
+DROP ROLE regress_fp_alter_role;
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 21ef1931f31..7ffba484739 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -3032,3 +3032,47 @@ INSERT INTO fp_deferred_pk VALUES (1);
 COMMIT;
 SELECT count(*) AS deferred_rows FROM fp_deferred_fk;  -- 1, check passed at commit
 DROP TABLE fp_deferred_fk, fp_deferred_pk;
+
+-- ALTER TABLE ... ADD FOREIGN KEY run from inside an AFTER trigger.  The
+-- validation scan must check each row itself rather than batching, so the
+-- orphan row is caught, the ALTER fails, and the constraint is never created.
+--
+-- Row-level security on the referenced table forces the row-at-a-time path;
+-- RI_Initial_Check() cannot use its single query when RLS applies.
+CREATE ROLE regress_fp_alter_role;
+CREATE TABLE fp_alter_pk (id int PRIMARY KEY);
+INSERT INTO fp_alter_pk VALUES (1);
+ALTER TABLE fp_alter_pk ENABLE ROW LEVEL SECURITY;
+CREATE POLICY fp_alter_pk_all ON fp_alter_pk USING (true);
+GRANT REFERENCES, SELECT ON fp_alter_pk TO regress_fp_alter_role;
+
+CREATE TABLE fp_alter_fk (a int);
+INSERT INTO fp_alter_fk VALUES (1), (999);
+ALTER TABLE fp_alter_fk OWNER TO regress_fp_alter_role;
+
+CREATE TABLE fp_alter_outer (a int);
+ALTER TABLE fp_alter_outer OWNER TO regress_fp_alter_role;
+
+CREATE FUNCTION fp_alter_from_trigger() RETURNS trigger
+LANGUAGE plpgsql AS $$
+BEGIN
+    BEGIN
+        EXECUTE 'ALTER TABLE fp_alter_fk ADD CONSTRAINT fp_alter_bad_fk '
+                'FOREIGN KEY (a) REFERENCES fp_alter_pk(id)';
+    EXCEPTION WHEN others THEN
+        RAISE;
+    END;
+    RETURN NEW;
+END$$;
+CREATE TRIGGER fp_alter_trg AFTER INSERT ON fp_alter_outer
+    FOR EACH ROW EXECUTE FUNCTION fp_alter_from_trigger();
+
+SET ROLE regress_fp_alter_role;
+INSERT INTO fp_alter_outer VALUES (1);
+RESET ROLE;
+SELECT conname, convalidated FROM pg_constraint WHERE conname = 'fp_alter_bad_fk';
+TABLE fp_alter_fk;
+
+DROP TABLE fp_alter_outer, fp_alter_fk, fp_alter_pk;
+DROP FUNCTION fp_alter_from_trigger();
+DROP ROLE regress_fp_alter_role;
-- 
2.47.3

