From ab5719ab432d6df51459b4b655064903b4841209 Mon Sep 17 00:00:00 2001 From: "Paul A. Jungwirth" Date: Wed, 17 Jun 2026 17:43:11 -0700 Subject: [PATCH v2] 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 capture the range column value just before the BEFORE UPDATE triggers fire, then compare it against the post-trigger value. If a trigger altered it, we raise an error. Discussion: https://www.postgresql.org/message-id/CA+renyVkfsrNNnYqLpf_g3mDV31KLFXAw-RRVmyLb7TcBLUO7A@mail.gmail.com --- src/backend/executor/nodeModifyTable.c | 164 ++++++++++++++++++- src/test/regress/expected/for_portion_of.out | 27 +++ src/test/regress/sql/for_portion_of.sql | 33 ++++ 3 files changed, 220 insertions(+), 4 deletions(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 33a6735f08d..8704cd37408 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -201,6 +201,14 @@ static void fireBSTriggers(ModifyTableState *node); static void fireASTriggers(ModifyTableState *node); static void ExecInitForPortionOf(ModifyTableState *mtstate, EState *estate, ResultRelInfo *resultRelInfo); +static Datum ExecForPortionOfSaveRange(ModifyTableContext *context, + ResultRelInfo *resultRelInfo, + TupleTableSlot *slot, + bool *isNull); +static void ExecForPortionOfCheckRange(ModifyTableContext *context, + ResultRelInfo *resultRelInfo, + TupleTableSlot *slot, + Datum origRange, bool origIsNull); /* @@ -2390,14 +2398,46 @@ ExecUpdatePrologue(ModifyTableContext *context, ResultRelInfo *resultRelInfo, if (resultRelInfo->ri_TrigDesc && resultRelInfo->ri_TrigDesc->trig_update_before_row) { + Node *forPortionOf = ((ModifyTable *) context->mtstate->ps.plan)->forPortionOf; + Datum origRange = (Datum) 0; + bool origIsNull = false; + bool proceed; + /* Flush any pending inserts, so rows are visible to the triggers */ if (context->estate->es_insert_pending_result_relations != NIL) ExecPendingInserts(context->estate); - return ExecBRUpdateTriggers(context->estate, context->epqstate, - resultRelInfo, tupleid, oldtuple, slot, - result, &context->tmfd, - context->mtstate->operation == CMD_MERGE); + /* + * With FOR PORTION OF, we must forbid triggers from changing the + * application time column, just as users can't SET it. Capture + * the current value, run triggers, then check below for changes. + */ + if (forPortionOf) + origRange = ExecForPortionOfSaveRange(context, resultRelInfo, slot, + &origIsNull); + + proceed = ExecBRUpdateTriggers(context->estate, context->epqstate, + resultRelInfo, tupleid, oldtuple, slot, + result, &context->tmfd, + context->mtstate->operation == CMD_MERGE); + + /* + * Check only when the trigger let the update proceed; either way, + * free the memory. + */ + if (forPortionOf) + { + ForPortionOfState *fpoState = resultRelInfo->ri_forPortionOf; + + if (proceed) + ExecForPortionOfCheckRange(context, resultRelInfo, slot, + origRange, origIsNull); + + if (!origIsNull && !fpoState->fp_leftoverstypcache->typbyval) + pfree(DatumGetPointer(origRange)); + } + + return proceed; } return true; @@ -5931,3 +5971,119 @@ ExecInitForPortionOf(ModifyTableState *mtstate, EState *estate, MemoryContextSwitchTo(oldcxt); } + +/* ---------------------------------------------------------------- + * ExecForPortionOfSaveRange + * + * Capture the FOR PORTION OF range column value from the new tuple slot + * just before BEFORE UPDATE triggers run. ExecForPortionOfCheckRange + * compares this against the post-trigger value to detect whether a + * trigger changed the range column, which is not allowed. + * ---------------------------------------------------------------- + */ +static Datum +ExecForPortionOfSaveRange(ModifyTableContext *context, + ResultRelInfo *resultRelInfo, + TupleTableSlot *slot, + bool *isNull) +{ + ModifyTableState *mtstate = context->mtstate; + ModifyTable *node = (ModifyTable *) mtstate->ps.plan; + ForPortionOfExpr *forPortionOf = (ForPortionOfExpr *) node->forPortionOf; + EState *estate = mtstate->ps.state; + ForPortionOfState *fpoState; + TypeCacheEntry *typcache; + MemoryContext oldcontext; + Datum saved; + + /* + * Lazily initialize the partition child's ForPortionOfState, like + * ExecForPortionOfLeftovers. The check will read fp_rangeAttno and the + * typcache from the same struct. + */ + if (!resultRelInfo->ri_forPortionOf) + ExecInitForPortionOf(mtstate, estate, resultRelInfo); + + fpoState = resultRelInfo->ri_forPortionOf; + + slot_getallattrs(slot); + + /* + * Look up the range's type cache entry, requesting the equality operator + * the check needs later. This is worth caching for the whole UPDATE. + */ + typcache = fpoState->fp_leftoverstypcache; + if (typcache == NULL) + { + typcache = lookup_type_cache(forPortionOf->rangeType, + TYPECACHE_EQ_OPR_FINFO); + fpoState->fp_leftoverstypcache = typcache; + } + + *isNull = slot->tts_isnull[fpoState->fp_rangeAttno - 1]; + if (*isNull) + return (Datum) 0; + + /* + * Copy the value into the query memory context so it survives the trigger + * invocation. The caller must release it after the check. + */ + oldcontext = MemoryContextSwitchTo(estate->es_query_cxt); + saved = datumCopy(slot->tts_values[fpoState->fp_rangeAttno - 1], + typcache->typbyval, typcache->typlen); + MemoryContextSwitchTo(oldcontext); + return saved; +} + +/* ---------------------------------------------------------------- + * ExecForPortionOfCheckRange + * + * Verify that BEFORE UPDATE triggers did not change the FOR PORTION OF + * range column. ExecForPortionOfSaveRange captured the value just before + * the triggers ran; here, right after they finish, we compare it against + * the current value and raise an error if a trigger altered it. + * ---------------------------------------------------------------- + */ +static void +ExecForPortionOfCheckRange(ModifyTableContext *context, + ResultRelInfo *resultRelInfo, + TupleTableSlot *slot, + Datum origRange, bool origIsNull) +{ + ModifyTableState *mtstate = context->mtstate; + ModifyTable *node = (ModifyTable *) mtstate->ps.plan; + ForPortionOfExpr *forPortionOf = (ForPortionOfExpr *) node->forPortionOf; + ForPortionOfState *fpoState = resultRelInfo->ri_forPortionOf; + TypeCacheEntry *typcache = fpoState->fp_leftoverstypcache; + bool newIsNull; + Datum newRange; + + /* + * ExecForPortionOfSaveRange always runs just before the triggers and + * populates the type cache entry, so it must be set by the time we get + * here. + */ + Assert(typcache != NULL); + + slot_getallattrs(slot); + newIsNull = slot->tts_isnull[fpoState->fp_rangeAttno - 1]; + newRange = slot->tts_values[fpoState->fp_rangeAttno - 1]; + + /* Compare with the default btree equality operator. */ + 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 != origIsNull || + (!newIsNull && + !DatumGetBool(FunctionCall2Coll(&typcache->eq_opr_finfo, + InvalidOid, + newRange, + origRange)))) + 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)); +} diff --git a/src/test/regress/expected/for_portion_of.out b/src/test/regress/expected/for_portion_of.out index 43408972117..e9afa4cff07 100644 --- a/src/test/regress/expected/for_portion_of.out +++ b/src/test/regress/expected/for_portion_of.out @@ -1843,6 +1843,33 @@ SELECT * FROM for_portion_of_test ORDER BY valid_at; DROP FUNCTION fpo_append_name_suffix CASCADE; NOTICE: drop cascades to trigger fpo_before_insert_row on table for_portion_of_test DROP TABLE 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 TABLE for_portion_of_test ( + id int4range, + valid_at daterange, + name text +); +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(); +INSERT INTO for_portion_of_test VALUES ('[1,2)', '[2010-01-01,2020-01-01)', 'foo'); +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(); +DROP TABLE for_portion_of_test; -- 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 7b08f8cf45e..6a50afcf7bb 100644 --- a/src/test/regress/sql/for_portion_of.sql +++ b/src/test/regress/sql/for_portion_of.sql @@ -1215,6 +1215,39 @@ SELECT * FROM for_portion_of_test ORDER BY valid_at; DROP FUNCTION fpo_append_name_suffix CASCADE; DROP TABLE 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 TABLE for_portion_of_test ( + id int4range, + valid_at daterange, + name text +); + +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(); + +INSERT INTO for_portion_of_test VALUES ('[1,2)', '[2010-01-01,2020-01-01)', 'foo'); + +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(); +DROP TABLE for_portion_of_test; + -- Test with multiranges CREATE TABLE for_portion_of_test2 ( -- 2.47.3