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 <sergei.patiakin@enterprisedb.com>
Reviewed-by: Paul A Jungwirth <pj@illuminatedcomputing.com>
Discussion: https://www.postgresql.org/message-id/flat/CANE55rCqcse_pwXBMWhbj3_7XROb8Dks6%3DOLFmKy3bO3zDsCsg%40mail.gmail.com

diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 478cb01783c3b7e0529ecc2042b0dbf1ee8c652c..b796f6e08018198f74e973af132292f0b8be0780 100644 (file)
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1601,6 +1601,18 @@ ExecForPortionOfLeftovers(ModifyTableContext *context,
 
                        didInit = true;
                }
+               else
+               {
+                       /*
+                        * Re-copy the original row into leftoverSlot because ExecInsert
+                        * might 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 0c0a205c44b6ae6b93074f644727b5742dc7f5f9..16b2f998dc0ccf1b4343ae38111190d81db55408 100644 (file)
--- a/src/test/regress/expected/for_portion_of.out
+++ b/src/test/regress/expected/for_portion_of.out
@@ -1793,6 +1793,44 @@ 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
+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 fd79a9b78e739252252bd3af7c7bb806fd9a64f8..63642b1851e3581ad9a5b26fc11a304ece4d8179 100644 (file)
--- a/src/test/regress/sql/for_portion_of.sql
+++ b/src/test/regress/sql/for_portion_of.sql
@@ -1169,6 +1169,44 @@ 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;
+DROP TABLE for_portion_of_test;
+
 -- Test with multiranges
 
 CREATE TABLE for_portion_of_test2 (