On Tue, Apr 21, 2026 at 2:51 AM jian he <[email protected]> wrote:
>
> On Sun, Apr 19, 2026 at 7:18 AM Paul A Jungwirth
> <[email protected]> wrote:
> >
> > Here is a patch that forbids changing the valid_at column in a BEFORE
> > trigger. It works by capturing the value before triggers run, then
> > checking afterwards if it is still the same (using the default btree
> > equality operator; probably a simple binary comparison is good
> > enough).
> >
> > ...
> The above works as expected, but the below is not what i expected.
>
> create type textrange as range (subtype = text, collation = "C");
> CREATE OR REPLACE FUNCTION trg_fpo()
> RETURNS TRIGGER LANGUAGE plpgsql AS
> $$
> BEGIN
> NEW.valid_at = '[A,d)';
> raise notice 'old: %, new: %', old, new;
> RETURN NEW;
> END;
> $$;
>
> create table fpo1(valid_at textrange, b int);
> CREATE TRIGGER fpo_before_update_row BEFORE UPDATE ON fpo1 FOR EACH
> ROW EXECUTE PROCEDURE trg_fpo();
> insert into fpo1 values ('[a,d]', 1);
>
> UPDATE fpo1 FOR PORTION OF valid_at FROM 'A' TO 'd' SET b = 2;
> NOTICE: old: ("[a,d]",1), new: ("[A,d)",2)
> ERROR: cannot change column "valid_at" from a BEFORE trigger because
> it is used in FOR PORTION OF
>
> Should I expect this to work without error, just like the table fpo3
> UPDATE FOR PORTION OF statement above?
That looks correct to me. In the C collation, uppercase letters come
before lowercase.
The row started as '[a,d)'. Then FOR PORTION OF valid_at FROM 'A' to
'd' leaves it as '[a,d)'
(because the intersection can only narrow). Then your BEFORE trigger
changes it to '[A,d)'.
Here is a new patch though, rebased and improved in a couple ways.
First, we are able to use ExecInitForPortionOf, added by another fix.
This reduces a lot of code duplication.
Also I moved the check out of ExecForPortionOfLeftovers. Now the time
between capturing the pre-trigger value and checking it is shorter: we
do the check right after triggers fire. This also means we don't have
to add fields to ForPortionOfState.
Yours,
--
Paul ~{:-)
[email protected]
From ab5719ab432d6df51459b4b655064903b4841209 Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <[email protected]>
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-rrvmylb7tcblu...@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
