From d1f93cbc5018c41b0948ece5eade08583afe6ae3 Mon Sep 17 00:00:00 2001 From: "Paul A. Jungwirth" 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