On Mon, 20 Jul 2026 at 17:21, Paul A Jungwirth <[email protected]> wrote: > > On Mon, Jul 20, 2026 at 6:17 AM Chao Li <[email protected]> wrote: > > > > PFA v2 with the new implementation. I tried to run check-world and it > > passed. > > Nice catch. I took a look at the v2 patch. It looks like a good fix to > me. I'm happy that we're testing cross-partition updates as well. > Thanks for the quick fix!
Sorry for the delay getting back to this. The fix looks good to me, and the point about cross-partition updates is a good one. Attached is a v3 patch, in which I reworked the comments a bit, and moved the logic of the new check to the place it's actually used. I also changed the tests to use a NOTICE-raising function instead of a sequence, since that makes it clearer which rows are/were being processed by the RETURNING clause. Regards, Dean
From 4cc6cf3a6f7c6652ebae9873482fc1d895899add Mon Sep 17 00:00:00 2001 From: Dean Rasheed <[email protected]> Date: Sat, 25 Jul 2026 10:56:50 +0100 Subject: [PATCH v3] Avoid RETURNING side effects for FOR PORTION OF leftovers. UPDATE/DELETE ... FOR PORTION OF inserts leftover rows for the untouched parts of the original row. These hidden inserts should not affect the command tag or ROW_COUNT, so they call ExecInsert() with canSetTag set to false. However, ExecInsert() still processed the RETURNING list whenever the target ResultRelInfo had ri_projectReturning set. That caused RETURNING expressions to be evaluated for leftover rows even though their results were discarded. As a result, expressions with side effects and information-leaking functions could be executed on the leftover rows, in addition to the visibly updated or deleted row. Fix by having ExecInsert() skip RETURNING processing when it is handling an internal FOR PORTION OF leftover insert. Use both the presence of a FOR PORTION OF clause and mtstate->operation == CMD_INSERT for this check, so that the auxiliary INSERT of a cross-partition UPDATE with a FOR PORTION OF clause still process RETURNING normally. Back-patch to v19, where support for FOR PORTION OF was added. Author: Chao Li <[email protected]> Reviewed-by: Dean Rasheed <[email protected]> Reviewed-by: Paul A Jungwirth <[email protected]> Discussion: https://postgr.es/m/[email protected] Backpatch-through: 19 --- src/backend/executor/nodeModifyTable.c | 14 ++++++-- src/test/regress/expected/for_portion_of.out | 36 +++++++++++++++----- src/test/regress/sql/for_portion_of.sql | 16 +++++++-- 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index b9781eb3b95..29f2cb3671d 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1335,8 +1335,18 @@ ExecInsert(ModifyTableContext *context, if (resultRelInfo->ri_WithCheckOptions != NIL) ExecWithCheckOptions(WCO_VIEW_CHECK, resultRelInfo, slot, estate); - /* Process RETURNING if present */ - if (resultRelInfo->ri_projectReturning) + /* + * Process RETURNING if present. + * + * If this is an UPDATE/DELETE ... FOR PORTION OF, we do not return the + * leftover rows inserted by ExecForPortionOfLeftovers(). Note that we + * must check mtstate->operation here, because we *do* want to process the + * newly inserted row of a cross-partition UPDATE with a FOR PORTION OF + * clause (ExecCrossPartitionUpdate() leaves mtstate->operation set to + * CMD_UPDATE, whereas ExecForPortionOfLeftovers() sets it to CMD_INSERT). + */ + if (resultRelInfo->ri_projectReturning && + !(node->forPortionOf && mtstate->operation == CMD_INSERT)) { TupleTableSlot *oldSlot = NULL; diff --git a/src/test/regress/expected/for_portion_of.out b/src/test/regress/expected/for_portion_of.out index 0e217f104ef..39a75efb7e7 100644 --- a/src/test/regress/expected/for_portion_of.out +++ b/src/test/regress/expected/for_portion_of.out @@ -920,14 +920,23 @@ SELECT * FROM for_portion_of_test ORDER BY id, valid_at; \set QUIET true -- UPDATE ... RETURNING returns only the updated values -- (not the inserted side values, which are added by a separate "statement"): +CREATE FUNCTION fpo_returning_row(text) +RETURNS text LANGUAGE plpgsql AS +$$ +BEGIN + RAISE NOTICE 'RETURNING %', $1; + RETURN $1; +END; +$$; UPDATE for_portion_of_test FOR PORTION OF valid_at FROM '2018-02-01' TO '2018-02-15' SET name = 'three^3' WHERE id = '[3,4)' - RETURNING *; - id | valid_at | name --------+-------------------------+--------- - [3,4) | [2018-02-01,2018-02-15) | three^3 + RETURNING *, fpo_returning_row(for_portion_of_test::text); +NOTICE: RETURNING ("[3,4)","[2018-02-01,2018-02-15)",three^3) + id | valid_at | name | fpo_returning_row +-------+-------------------------+---------+--------------------------------------------- + [3,4) | [2018-02-01,2018-02-15) | three^3 | ("[3,4)","[2018-02-01,2018-02-15)",three^3) (1 row) -- UPDATE ... RETURNING supports NEW and OLD valid_at @@ -975,10 +984,11 @@ DELETE FROM for_portion_of_test WHERE id = '[99,100)'; DELETE FROM for_portion_of_test FOR PORTION OF valid_at FROM '2018-02-02' TO '2018-02-03' WHERE id = '[3,4)' - RETURNING *; - id | valid_at | name --------+-------------------------+--------- - [3,4) | [2018-02-01,2018-02-10) | three^3 + RETURNING *, fpo_returning_row(for_portion_of_test::text); +NOTICE: RETURNING ("[3,4)","[2018-02-01,2018-02-10)",three^3) + id | valid_at | name | fpo_returning_row +-------+-------------------------+---------+--------------------------------------------- + [3,4) | [2018-02-01,2018-02-10) | three^3 | ("[3,4)","[2018-02-01,2018-02-10)",three^3) (1 row) -- DELETE FOR PORTION OF in a PL/pgSQL function @@ -2137,7 +2147,14 @@ UPDATE temporal_partitioned FOR PORTION OF valid_at FROM '2000-03-01' TO '2000-0 UPDATE temporal_partitioned FOR PORTION OF valid_at FROM '2000-06-01' TO '2000-07-01' SET name = 'one^2', id = '[4,5)' - WHERE id = '[1,2)'; + WHERE id = '[1,2)' + RETURNING id, valid_at, name, fpo_returning_row(temporal_partitioned::text); +NOTICE: RETURNING ("[4,5)","[2000-06-01,2000-07-01)",one^2,30) + id | valid_at | name | fpo_returning_row +-------+-------------------------+-------+---------------------------------------------- + [4,5) | [2000-06-01,2000-07-01) | one^2 | ("[4,5)","[2000-06-01,2000-07-01)",one^2,30) +(1 row) + -- Move from partition 3 to partition 1 UPDATE temporal_partitioned FOR PORTION OF valid_at FROM '2000-06-01' TO '2000-07-01' SET name = 'three^2', @@ -2199,6 +2216,7 @@ SELECT * FROM temporal_partitioned_5 ORDER BY id, valid_at; five | [2000-07-01,2010-01-01) | [5,6) | 3471 (4 rows) +DROP FUNCTION fpo_returning_row; DROP TABLE temporal_partitioned; -- UPDATE/DELETE FOR PORTION OF with RULEs CREATE TABLE fpo_rule (f1 bigint, f2 int4range); diff --git a/src/test/regress/sql/for_portion_of.sql b/src/test/regress/sql/for_portion_of.sql index a8d29a76b22..17d150e5fc8 100644 --- a/src/test/regress/sql/for_portion_of.sql +++ b/src/test/regress/sql/for_portion_of.sql @@ -590,11 +590,19 @@ SELECT * FROM for_portion_of_test ORDER BY id, valid_at; -- UPDATE ... RETURNING returns only the updated values -- (not the inserted side values, which are added by a separate "statement"): +CREATE FUNCTION fpo_returning_row(text) +RETURNS text LANGUAGE plpgsql AS +$$ +BEGIN + RAISE NOTICE 'RETURNING %', $1; + RETURN $1; +END; +$$; UPDATE for_portion_of_test FOR PORTION OF valid_at FROM '2018-02-01' TO '2018-02-15' SET name = 'three^3' WHERE id = '[3,4)' - RETURNING *; + RETURNING *, fpo_returning_row(for_portion_of_test::text); -- UPDATE ... RETURNING supports NEW and OLD valid_at UPDATE for_portion_of_test @@ -629,7 +637,7 @@ DELETE FROM for_portion_of_test WHERE id = '[99,100)'; DELETE FROM for_portion_of_test FOR PORTION OF valid_at FROM '2018-02-02' TO '2018-02-03' WHERE id = '[3,4)' - RETURNING *; + RETURNING *, fpo_returning_row(for_portion_of_test::text); -- DELETE FOR PORTION OF in a PL/pgSQL function INSERT INTO for_portion_of_test (id, valid_at, name) VALUES @@ -1439,7 +1447,8 @@ UPDATE temporal_partitioned FOR PORTION OF valid_at FROM '2000-03-01' TO '2000-0 UPDATE temporal_partitioned FOR PORTION OF valid_at FROM '2000-06-01' TO '2000-07-01' SET name = 'one^2', id = '[4,5)' - WHERE id = '[1,2)'; + WHERE id = '[1,2)' + RETURNING id, valid_at, name, fpo_returning_row(temporal_partitioned::text); -- Move from partition 3 to partition 1 UPDATE temporal_partitioned FOR PORTION OF valid_at FROM '2000-06-01' TO '2000-07-01' @@ -1460,6 +1469,7 @@ SELECT * FROM temporal_partitioned_1 ORDER BY id, valid_at; SELECT * FROM temporal_partitioned_3 ORDER BY id, valid_at; SELECT * FROM temporal_partitioned_5 ORDER BY id, valid_at; +DROP FUNCTION fpo_returning_row; DROP TABLE temporal_partitioned; -- UPDATE/DELETE FOR PORTION OF with RULEs -- 2.51.0
