v1-0001-Forbid-BEFORE-UPDATE-triggers-changing-the-FOR-PO.patch

From d1f93cbc5018c41b0948ece5eade08583afe6ae3 Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <pj@illuminatedcomputing.com>
Date: Thu, 16 Apr 2026 13:00:59 -0700
Subject: [PATCH v1] Forbid BEFORE UPDATE triggers changing the FOR PORTION OF
 column

Just as we forbid UPDATE t FOR PORTION OF valid_at ... SET valid_at, we
should forbid setting the application-time column with a BEFORE trigger.

We record the value before triggers fire, and then we compare
afterwards to make sure it hasn't been altered. If so we raise an error.
---
 src/backend/executor/nodeModifyTable.c       | 159 +++++++++++++++++--
 src/include/nodes/execnodes.h                |   5 +
 src/test/regress/expected/for_portion_of.out |  20 +++
 src/test/regress/sql/for_portion_of.sql      |  24 +++
 4 files changed, 196 insertions(+), 12 deletions(-)

diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index ef2a6bc6e9d..c82dea2eff1 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -171,7 +171,11 @@ static bool ExecOnConflictSelect(ModifyTableContext *context,
 static void ExecForPortionOfLeftovers(ModifyTableContext *context,
 									  EState *estate,
 									  ResultRelInfo *resultRelInfo,
-									  ItemPointer tupleid);
+									  ItemPointer tupleid,
+									  TupleTableSlot *newSlot);
+static void ExecForPortionOfSaveRange(ModifyTableContext *context,
+									  ResultRelInfo *resultRelInfo,
+									  TupleTableSlot *slot);
 static TupleTableSlot *ExecPrepareTupleRouting(ModifyTableState *mtstate,
 											   EState *estate,
 											   PartitionTupleRouting *proute,
@@ -1403,14 +1407,14 @@ static void
 ExecForPortionOfLeftovers(ModifyTableContext *context,
 						  EState *estate,
 						  ResultRelInfo *resultRelInfo,
-						  ItemPointer tupleid)
+						  ItemPointer tupleid,
+						  TupleTableSlot *newSlot)
 {
 	ModifyTableState *mtstate = context->mtstate;
 	ModifyTable *node = (ModifyTable *) mtstate->ps.plan;
 	ForPortionOfExpr *forPortionOf = (ForPortionOfExpr *) node->forPortionOf;
 	AttrNumber	rangeAttno;
 	Datum		oldRange;
-	TypeCacheEntry *typcache;
 	ForPortionOfState *fpoState;
 	TupleTableSlot *oldtupleSlot;
 	TupleTableSlot *leftoverSlot;
@@ -1490,15 +1494,51 @@ ExecForPortionOfLeftovers(ModifyTableContext *context,
 	oldRange = oldtupleSlot->tts_values[rangeAttno - 1];
 
 	/*
-	 * Get the range's type cache entry. This is worth caching for the whole
-	 * UPDATE/DELETE as range functions do.
+	 * If BEFORE UPDATE triggers fired, they might have changed the range
+	 * column, which would break the temporal semantics of FOR PORTION OF.
+	 * We captured the column value in ExecForPortionOfSaveRange, so now
+	 * compare it with the current value to detect tampering. This parallels
+	 * how in analysis we reject SETting the range column directly.
 	 */
-
-	typcache = fpoState->fp_leftoverstypcache;
-	if (typcache == NULL)
+	if (newSlot != NULL && fpoState->fp_origNewRangeValid)
 	{
-		typcache = lookup_type_cache(forPortionOf->rangeType, 0);
-		fpoState->fp_leftoverstypcache = typcache;
+		bool		newIsNull;
+		Datum		newRange;
+		TypeCacheEntry *typcache;
+
+		/*
+		 * Get the range's type cache entry. This is worth caching for the whole
+		 * UPDATE/DELETE as range functions do.
+		 */
+
+		typcache = fpoState->fp_leftoverstypcache;
+		if (typcache == NULL)
+		{
+			typcache = lookup_type_cache(forPortionOf->rangeType,
+										 TYPECACHE_EQ_OPR_FINFO);
+			fpoState->fp_leftoverstypcache = typcache;
+		}
+
+		slot_getallattrs(newSlot);
+		newIsNull = newSlot->tts_isnull[rangeAttno - 1];
+		newRange = newSlot->tts_values[rangeAttno - 1];
+
+		if (!OidIsValid(typcache->eq_opr_finfo.fn_oid))
+			ereport(ERROR,
+					errcode(ERRCODE_UNDEFINED_FUNCTION),
+					errmsg("could not identify an equality operator for type %s",
+						   format_type_be(forPortionOf->rangeType)));
+
+		if (newIsNull != fpoState->fp_origNewRangeIsNull ||
+			(!newIsNull &&
+			 !DatumGetBool(FunctionCall2Coll(&typcache->eq_opr_finfo,
+											 InvalidOid,
+											 newRange,
+											 fpoState->fp_origNewRange))))
+			ereport(ERROR,
+					errcode(ERRCODE_TRIGGERED_DATA_CHANGE_VIOLATION),
+					errmsg("cannot change column \"%s\" from a BEFORE trigger because it is used in FOR PORTION OF",
+						   forPortionOf->range_name));
 	}
 
 	/*
@@ -1617,6 +1657,92 @@ ExecForPortionOfLeftovers(ModifyTableContext *context,
 	}
 }
 
+/* ----------------------------------------------------------------
+ *		ExecForPortionOfSaveRange
+ *
+ *		Capture the FOR PORTION OF range column value from the new tuple
+ *		slot just before BEFORE UPDATE triggers run. ExecForPortionOfLeftovers
+ *		later compares the saved value with the post-trigger value to detect
+ *		whether a trigger changed the range column, which is not allowed.
+ * ----------------------------------------------------------------
+ */
+static void
+ExecForPortionOfSaveRange(ModifyTableContext *context,
+						  ResultRelInfo *resultRelInfo,
+						  TupleTableSlot *slot)
+{
+	ModifyTableState *mtstate = context->mtstate;
+	ModifyTable *node = (ModifyTable *) mtstate->ps.plan;
+	ForPortionOfExpr *forPortionOf = (ForPortionOfExpr *) node->forPortionOf;
+	ForPortionOfState *fpoState;
+	AttrNumber	rangeAttno;
+	TupleConversionMap *map = NULL;
+	MemoryContext oldcontext;
+
+	/*
+	 * Lazily initialize the partition child's ForPortionOfState, mirroring
+	 * ExecForPortionOfLeftovers so the saved value lives on the same struct
+	 * the check will read from.
+	 */
+	if (!resultRelInfo->ri_forPortionOf)
+	{
+		ForPortionOfState *leafState = makeNode(ForPortionOfState);
+		ForPortionOfState *rootFpoState;
+
+		if (!mtstate->rootResultRelInfo)
+			elog(ERROR, "no root relation but ri_forPortionOf is uninitialized");
+		rootFpoState = mtstate->rootResultRelInfo->ri_forPortionOf;
+		Assert(rootFpoState);
+
+		leafState->fp_rangeName = rootFpoState->fp_rangeName;
+		leafState->fp_rangeType = rootFpoState->fp_rangeType;
+		leafState->fp_rangeAttno = rootFpoState->fp_rangeAttno;
+		leafState->fp_targetRange = rootFpoState->fp_targetRange;
+		leafState->fp_Leftover = rootFpoState->fp_Leftover;
+		leafState->fp_Existing =
+			table_slot_create(resultRelInfo->ri_RelationDesc,
+							  &mtstate->ps.state->es_tupleTable);
+
+		resultRelInfo->ri_forPortionOf = leafState;
+	}
+	fpoState = resultRelInfo->ri_forPortionOf;
+
+	rangeAttno = forPortionOf->rangeVar->varattno;
+	if (resultRelInfo->ri_RootResultRelInfo)
+		map = ExecGetChildToRootMap(resultRelInfo);
+	if (map != NULL)
+		rangeAttno = map->attrMap->attnums[rangeAttno - 1];
+
+	slot_getallattrs(slot);
+
+	/* Release any value saved from a prior row. */
+	if (fpoState->fp_origNewRangeValid)
+	{
+		fpoState->fp_origNewRangeValid = false;
+		if (!fpoState->fp_origNewRangeIsNull)
+			pfree(DatumGetPointer(fpoState->fp_origNewRange));
+	}
+
+	if (slot->tts_isnull[rangeAttno - 1])
+	{
+		fpoState->fp_origNewRange = (Datum) 0;
+		fpoState->fp_origNewRangeIsNull = true;
+	}
+	else
+	{
+		/*
+		 * Make sure we copy everything for pass-by-reference types
+		 * (like range and multirange).
+		 */
+		oldcontext = MemoryContextSwitchTo(mtstate->ps.state->es_query_cxt);
+		fpoState->fp_origNewRange = datumCopy(slot->tts_values[rangeAttno - 1],
+											  false, -1);
+		MemoryContextSwitchTo(oldcontext);
+		fpoState->fp_origNewRangeIsNull = false;
+	}
+	fpoState->fp_origNewRangeValid = true;
+}
+
 /* ----------------------------------------------------------------
  *		ExecBatchInsert
  *
@@ -1810,7 +1936,8 @@ ExecDeleteEpilogue(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
 
 	/* Compute temporal leftovers in FOR PORTION OF */
 	if (((ModifyTable *) context->mtstate->ps.plan)->forPortionOf)
-		ExecForPortionOfLeftovers(context, estate, resultRelInfo, tupleid);
+		ExecForPortionOfLeftovers(context, estate, resultRelInfo, tupleid,
+								  NULL);
 
 	/* AFTER ROW DELETE Triggers */
 	ExecARDeleteTriggers(estate, resultRelInfo, tupleid, oldtuple,
@@ -2390,6 +2517,13 @@ ExecUpdatePrologue(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
 		if (context->estate->es_insert_pending_result_relations != NIL)
 			ExecPendingInserts(context->estate);
 
+		/*
+		 * For FOR PORTION OF, remember the range column value so we can
+		 * later detect whether a BEFORE trigger changed it.
+		 */
+		if (((ModifyTable *) context->mtstate->ps.plan)->forPortionOf)
+			ExecForPortionOfSaveRange(context, resultRelInfo, slot);
+
 		return ExecBRUpdateTriggers(context->estate, context->epqstate,
 									resultRelInfo, tupleid, oldtuple, slot,
 									result, &context->tmfd,
@@ -2615,7 +2749,8 @@ ExecUpdateEpilogue(ModifyTableContext *context, UpdateContext *updateCxt,
 
 	/* Compute temporal leftovers in FOR PORTION OF */
 	if (((ModifyTable *) context->mtstate->ps.plan)->forPortionOf)
-		ExecForPortionOfLeftovers(context, context->estate, resultRelInfo, tupleid);
+		ExecForPortionOfLeftovers(context, context->estate, resultRelInfo,
+								  tupleid, slot);
 
 	/* AFTER ROW UPDATE Triggers */
 	ExecARUpdateTriggers(context->estate, resultRelInfo,
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 13359180d25..efcd52411ab 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -483,6 +483,11 @@ typedef struct ForPortionOfState
 	TypeCacheEntry *fp_leftoverstypcache;	/* type cache entry of the range */
 	TupleTableSlot *fp_Existing;	/* slot to store old tuple */
 	TupleTableSlot *fp_Leftover;	/* slot to store leftover */
+	Datum		fp_origNewRange;	/* range column value captured just before
+									 * BEFORE UPDATE triggers fire, so we can
+									 * detect whether they changed it */
+	bool		fp_origNewRangeIsNull;
+	bool		fp_origNewRangeValid;	/* is fp_origNewRange meaningful? */
 } ForPortionOfState;
 
 /*
diff --git a/src/test/regress/expected/for_portion_of.out b/src/test/regress/expected/for_portion_of.out
index 31f772c723d..f9b9c1d0d6d 100644
--- a/src/test/regress/expected/for_portion_of.out
+++ b/src/test/regress/expected/for_portion_of.out
@@ -1793,6 +1793,26 @@ SELECT * FROM for_portion_of_test WHERE id = '[4,5)' ORDER BY id, valid_at;
 (3 rows)
 
 DROP TRIGGER fpo_after_delete_row ON for_portion_of_test;
+-- A BEFORE UPDATE trigger that changes the application-time column must
+-- raise an error, just as an explicit SET on that column does.
+CREATE FUNCTION trg_fpo_change_valid_at()
+RETURNS TRIGGER LANGUAGE plpgsql AS
+$$
+BEGIN
+  NEW.valid_at = daterange('2018-01-01', '2019-01-01');
+  RETURN NEW;
+END;
+$$;
+CREATE TRIGGER fpo_before_update_row
+  BEFORE UPDATE ON for_portion_of_test
+  FOR EACH ROW EXECUTE PROCEDURE trg_fpo_change_valid_at();
+UPDATE for_portion_of_test
+  FOR PORTION OF valid_at FROM '2018-05-01' TO '2018-06-01'
+  SET name = CONCAT(name, '!')
+  WHERE id = '[1,2)';
+ERROR:  cannot change column "valid_at" from a BEFORE trigger because it is used in FOR PORTION OF
+DROP TRIGGER fpo_before_update_row ON for_portion_of_test;
+DROP FUNCTION trg_fpo_change_valid_at();
 -- Test with multiranges
 CREATE TABLE for_portion_of_test2 (
   id int4range NOT NULL,
diff --git a/src/test/regress/sql/for_portion_of.sql b/src/test/regress/sql/for_portion_of.sql
index d4062acf1d1..39bb17a9409 100644
--- a/src/test/regress/sql/for_portion_of.sql
+++ b/src/test/regress/sql/for_portion_of.sql
@@ -1169,6 +1169,30 @@ SELECT * FROM for_portion_of_test WHERE id = '[4,5)' ORDER BY id, valid_at;
 
 DROP TRIGGER fpo_after_delete_row ON for_portion_of_test;
 
+-- A BEFORE UPDATE trigger that changes the application-time column must
+-- raise an error, just as an explicit SET on that column does.
+
+CREATE FUNCTION trg_fpo_change_valid_at()
+RETURNS TRIGGER LANGUAGE plpgsql AS
+$$
+BEGIN
+  NEW.valid_at = daterange('2018-01-01', '2019-01-01');
+  RETURN NEW;
+END;
+$$;
+
+CREATE TRIGGER fpo_before_update_row
+  BEFORE UPDATE ON for_portion_of_test
+  FOR EACH ROW EXECUTE PROCEDURE trg_fpo_change_valid_at();
+
+UPDATE for_portion_of_test
+  FOR PORTION OF valid_at FROM '2018-05-01' TO '2018-06-01'
+  SET name = CONCAT(name, '!')
+  WHERE id = '[1,2)';
+
+DROP TRIGGER fpo_before_update_row ON for_portion_of_test;
+DROP FUNCTION trg_fpo_change_valid_at();
+
 -- Test with multiranges
 
 CREATE TABLE for_portion_of_test2 (
-- 
2.47.3