On Tue, May 5, 2026 at 2:16 PM Zsolt Parragi <[email protected]> wrote:
>
> + /*
> + * Re-copy the original row into leftoverSlot because
> + * ExecInsert may pass leftoverSlot to BEFORE INSERT
> + * triggers, which can modify the slot contents.
> + */
>
> Shouldn't this mention BEFORE ROW INSERT triggers? Everywhere else the
> comments/commit message/test is specific about this.
I agree being specific is helpful. Here is an updated patch.
Yours,
--
Paul ~{:-)
[email protected]
From c96fa8b27fefa4d44824fb944951331dcb32834d Mon Sep 17 00:00:00 2001
From: Sergei Patiakin <[email protected]>
Date: Wed, 8 Apr 2026 14:46:09 +0200
Subject: [PATCH v2] Fix cross-leftover pollution in FOR PORTION OF insert
triggers
When we insert temporal leftovers after an UPDATE FOR PORTION OF, we must make a
new copy of the tuple before each insert. Otherwise if an insert trigger assigns
to attributes of NEW, the second leftover sees those changes.
Author: Sergei Patiakin <[email protected]>
Discussion: https://postgr.es/m/CANE55rCqcse_pwXBMWhbj3_7XROb8Dks6%3DOLFmKy3bO3zDsCsg%40mail.gmail.com
---
src/backend/executor/nodeModifyTable.c | 13 +++++++
src/test/regress/expected/for_portion_of.out | 37 ++++++++++++++++++++
src/test/regress/sql/for_portion_of.sql | 37 ++++++++++++++++++++
3 files changed, 87 insertions(+)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 4cb057ca4f9..123e97c66b5 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1600,6 +1600,19 @@ ExecForPortionOfLeftovers(ModifyTableContext *context,
didInit = true;
}
+ else
+ {
+ /*
+ * Re-copy the original row into leftoverSlot because
+ * ExecInsert may pass leftoverSlot to BEFORE ROW INSERT
+ * triggers, which can modify the slot contents.
+ */
+ if (map != NULL)
+ execute_attr_map_slot(map->attrMap, oldtupleSlot,
+ leftoverSlot);
+ else
+ ExecForceStoreHeapTuple(oldtuple, leftoverSlot, false);
+ }
leftoverSlot->tts_values[forPortionOf->rangeVar->varattno - 1] = leftover;
leftoverSlot->tts_isnull[forPortionOf->rangeVar->varattno - 1] = false;
diff --git a/src/test/regress/expected/for_portion_of.out b/src/test/regress/expected/for_portion_of.out
index 0c0a205c44b..14011239bee 100644
--- a/src/test/regress/expected/for_portion_of.out
+++ b/src/test/regress/expected/for_portion_of.out
@@ -1793,6 +1793,43 @@ 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;
+-- Test that a tuple-modifying BEFORE INSERT ROW trigger acts
+-- consistently on both temporal leftovers.
+-- When FOR PORTION OF splits a row into two leftovers,
+-- both triggers should get the original row's values.
+DROP TABLE for_portion_of_test;
+CREATE TABLE for_portion_of_test (
+ id int,
+ valid_at daterange,
+ name text
+);
+CREATE FUNCTION fpo_append_name_suffix()
+RETURNS TRIGGER LANGUAGE plpgsql AS
+$$
+BEGIN
+ NEW.name := NEW.name || '+insert';
+ RETURN NEW;
+END;
+$$;
+CREATE TRIGGER fpo_before_insert_row
+ BEFORE INSERT ON for_portion_of_test
+ FOR EACH ROW EXECUTE PROCEDURE fpo_append_name_suffix();
+INSERT INTO for_portion_of_test VALUES (1, '[2020-01-01,2020-12-31)', 'foo');
+UPDATE for_portion_of_test
+ FOR PORTION OF valid_at FROM '2020-04-01' TO '2020-08-01'
+ SET name = 'bar'
+ WHERE id = 1;
+-- Both leftovers should have the same name: 'foo+insert+insert'.
+SELECT * FROM for_portion_of_test ORDER BY valid_at;
+ id | valid_at | name
+----+-------------------------+-------------------
+ 1 | [2020-01-01,2020-04-01) | foo+insert+insert
+ 1 | [2020-04-01,2020-08-01) | bar
+ 1 | [2020-08-01,2020-12-31) | foo+insert+insert
+(3 rows)
+
+DROP FUNCTION fpo_append_name_suffix CASCADE;
+NOTICE: drop cascades to trigger fpo_before_insert_row on 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 fd79a9b78e7..f39a95fd10e 100644
--- a/src/test/regress/sql/for_portion_of.sql
+++ b/src/test/regress/sql/for_portion_of.sql
@@ -1169,6 +1169,43 @@ 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;
+-- Test that a tuple-modifying BEFORE INSERT ROW trigger acts
+-- consistently on both temporal leftovers.
+-- When FOR PORTION OF splits a row into two leftovers,
+-- both triggers should get the original row's values.
+
+DROP TABLE for_portion_of_test;
+CREATE TABLE for_portion_of_test (
+ id int,
+ valid_at daterange,
+ name text
+);
+
+CREATE FUNCTION fpo_append_name_suffix()
+RETURNS TRIGGER LANGUAGE plpgsql AS
+$$
+BEGIN
+ NEW.name := NEW.name || '+insert';
+ RETURN NEW;
+END;
+$$;
+
+CREATE TRIGGER fpo_before_insert_row
+ BEFORE INSERT ON for_portion_of_test
+ FOR EACH ROW EXECUTE PROCEDURE fpo_append_name_suffix();
+
+INSERT INTO for_portion_of_test VALUES (1, '[2020-01-01,2020-12-31)', 'foo');
+
+UPDATE for_portion_of_test
+ FOR PORTION OF valid_at FROM '2020-04-01' TO '2020-08-01'
+ SET name = 'bar'
+ WHERE id = 1;
+
+-- Both leftovers should have the same name: 'foo+insert+insert'.
+SELECT * FROM for_portion_of_test ORDER BY valid_at;
+
+DROP FUNCTION fpo_append_name_suffix CASCADE;
+
-- Test with multiranges
CREATE TABLE for_portion_of_test2 (
--
2.47.3
