From 81297d846c74fd3da79ac206550b79ddacc168fa Mon Sep 17 00:00:00 2001 From: "Paul A. Jungwirth" Date: Mon, 31 Aug 2026 09:16:53 -0700 Subject: [PATCH v1] Fix assertion failures in DELETE FOR PORTION OF tuple routing We must handle cases where an inserted temporal leftover gets routed to a different partition than the original tuple's. This happens if the partition key depends on the application-time column used in the FOR PORTION OF clause. Everything works for UPDATE, but for DELETE we can hit some Asserts, since that operation never needed to be covered before. One Assert is for the RETURNING list; the other, for WITH CHECK OPTION. This commit updates both to include CMD_DELETE. Author: Paul A Jungwirth Reported-by: Peter Eisentraut Discussion: https://postgr.es/m/03276924-6d55-470d-8192-a49c053c940f@eisentraut.org --- src/backend/executor/execPartition.c | 22 +++++-- src/test/regress/expected/for_portion_of.out | 59 +++++++++++++++++++ src/test/regress/expected/updatable_views.out | 45 ++++++++++++++ src/test/regress/sql/for_portion_of.sql | 42 +++++++++++++ src/test/regress/sql/updatable_views.sql | 33 +++++++++++ 5 files changed, 195 insertions(+), 6 deletions(-) diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 33ec5bfde4c..0a581d719bd 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -591,7 +591,8 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, /* * Verify result relation is a valid target for an INSERT. An UPDATE of a * partition-key becomes a DELETE+INSERT operation, so this check is still - * required when the operation is CMD_UPDATE. + * required when the operation is CMD_UPDATE. It is also required for + * CMD_DELETE, because DELETE ... FOR PORTION OF inserts leftover rows. */ CheckValidResultRel(leaf_part_rri, CMD_INSERT, node ? node->onConflictAction : ONCONFLICT_NONE, NIL, node); @@ -612,8 +613,9 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, * Build WITH CHECK OPTION constraints for the partition. Note that we * didn't build the withCheckOptionList for partitions within the planner, * but simple translation of varattnos will suffice. This only occurs for - * the INSERT case or in the case of UPDATE/MERGE tuple routing where we - * didn't find a result rel to reuse. + * the INSERT case or in the case of UPDATE/DELETE/MERGE tuple routing + * where we didn't find a result rel to reuse. We reach here with DELETE + * only when inserting temporal leftovers. */ if (node && node->withCheckOptionLists != NIL) { @@ -624,7 +626,8 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, /* * In the case of INSERT on a partitioned table, there is only one * plan. Likewise, there is only one WCO list, not one per partition. - * For UPDATE/MERGE, there are as many WCO lists as there are plans. + * For UPDATE/DELETE/MERGE, there are as many WCO lists as there are + * plans. */ Assert((node->operation == CMD_INSERT && list_length(node->withCheckOptionLists) == 1 && @@ -632,6 +635,9 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, (node->operation == CMD_UPDATE && list_length(node->withCheckOptionLists) == list_length(node->resultRelations)) || + (node->operation == CMD_DELETE && + list_length(node->withCheckOptionLists) == + list_length(node->resultRelations)) || (node->operation == CMD_MERGE && list_length(node->withCheckOptionLists) == list_length(node->resultRelations))); @@ -679,8 +685,9 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, * Build the RETURNING projection for the partition. Note that we didn't * build the returningList for partitions within the planner, but simple * translation of varattnos will suffice. This only occurs for the INSERT - * case or in the case of UPDATE/MERGE tuple routing where we didn't find - * a result rel to reuse. + * case or in the case of UPDATE/DELETE/MERGE tuple routing where we + * didn't find a result rel to reuse. We reach here with DELETE only when + * inserting temporal leftovers. */ if (node && node->returningLists != NIL) { @@ -695,6 +702,9 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, (node->operation == CMD_UPDATE && list_length(node->returningLists) == list_length(node->resultRelations)) || + (node->operation == CMD_DELETE && + list_length(node->returningLists) == + list_length(node->resultRelations)) || (node->operation == CMD_MERGE && list_length(node->returningLists) == list_length(node->resultRelations))); diff --git a/src/test/regress/expected/for_portion_of.out b/src/test/regress/expected/for_portion_of.out index 1a53e549de0..64789d1777b 100644 --- a/src/test/regress/expected/for_portion_of.out +++ b/src/test/regress/expected/for_portion_of.out @@ -2218,6 +2218,65 @@ SELECT * FROM temporal_partitioned_5 ORDER BY id, valid_at; DROP FUNCTION fpo_returning_row; DROP TABLE temporal_partitioned; +-- Test FOR PORTION OF when the partition key depends on the range column. +-- Then a leftover can belong to a partition that is not one of the plan's +-- result relations, and we must build a new ResultRelInfo for it. +CREATE TABLE temporal_partitioned_by_valid_at ( + id int4range, + valid_at daterange, + name text +) PARTITION BY RANGE (lower(valid_at)); +CREATE TABLE temporal_partitioned_early + PARTITION OF temporal_partitioned_by_valid_at + FOR VALUES FROM (MINVALUE) TO ('2000-06-01'); +CREATE TABLE temporal_partitioned_late + PARTITION OF temporal_partitioned_by_valid_at + FOR VALUES FROM ('2000-06-01') TO (MAXVALUE); +INSERT INTO temporal_partitioned_by_valid_at (id, valid_at, name) VALUES + ('[1,2)', daterange('2000-01-01', '2010-01-01'), 'one'); +-- The WHERE clause prunes away the late partition, but the second leftover +-- belongs there. The leftovers must not appear in the RETURNING output. +DELETE FROM temporal_partitioned_by_valid_at + FOR PORTION OF valid_at FROM '2000-03-01' TO '2000-07-01' + WHERE lower(valid_at) < '2000-06-01' + RETURNING id, valid_at, name; + id | valid_at | name +-------+-------------------------+------ + [1,2) | [2000-01-01,2010-01-01) | one +(1 row) + +SELECT tableoid::regclass, * FROM temporal_partitioned_by_valid_at + ORDER BY id, valid_at; + tableoid | id | valid_at | name +----------------------------+-------+-------------------------+------ + temporal_partitioned_early | [1,2) | [2000-01-01,2000-03-01) | one + temporal_partitioned_late | [1,2) | [2000-07-01,2010-01-01) | one +(2 rows) + +-- The same thing for UPDATE +DELETE FROM temporal_partitioned_by_valid_at; +INSERT INTO temporal_partitioned_by_valid_at (id, valid_at, name) VALUES + ('[1,2)', daterange('2000-01-01', '2010-01-01'), 'one'); +UPDATE temporal_partitioned_by_valid_at + FOR PORTION OF valid_at FROM '2000-03-01' TO '2000-07-01' + SET name = 'one^1' + WHERE lower(valid_at) < '2000-06-01' + RETURNING id, valid_at, name; + id | valid_at | name +-------+-------------------------+------- + [1,2) | [2000-03-01,2000-07-01) | one^1 +(1 row) + +SELECT tableoid::regclass, * FROM temporal_partitioned_by_valid_at + ORDER BY id, valid_at; + tableoid | id | valid_at | name +----------------------------+-------+-------------------------+------- + temporal_partitioned_early | [1,2) | [2000-01-01,2000-03-01) | one + temporal_partitioned_early | [1,2) | [2000-03-01,2000-07-01) | one^1 + temporal_partitioned_late | [1,2) | [2000-07-01,2010-01-01) | one +(3 rows) + +DROP TABLE temporal_partitioned_by_valid_at; -- UPDATE/DELETE FOR PORTION OF with RULEs CREATE TABLE fpo_rule (f1 bigint, f2 int4range); INSERT INTO fpo_rule VALUES (1, '[1, 11)'); diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out index b4b4e93a7dd..5f9adf91029 100644 --- a/src/test/regress/expected/updatable_views.out +++ b/src/test/regress/expected/updatable_views.out @@ -3824,6 +3824,51 @@ select * from uv_fpo_wco_tab order by valid_at; drop view uv_fpo_wco_view; drop view uv_fpo_nowco_view; drop table uv_fpo_wco_tab; +-- On a partitioned table a leftover can be routed to a partition that is not +-- one of the plan's result relations. WITH CHECK OPTION must be enforced +-- there too. +create table uv_fpo_wco_part (id int4range, valid_at daterange, b int) + partition by range (lower(valid_at)); +create table uv_fpo_wco_part1 partition of uv_fpo_wco_part + for values from (minvalue) to ('2000-06-01'); +create table uv_fpo_wco_part2 partition of uv_fpo_wco_part + for values from ('2000-06-01') to (maxvalue); +insert into uv_fpo_wco_part values ('[1,1]', '[2000-01-01,2004-01-01)', 0); +-- This view accepts both leftovers. The where clause prunes away part2, but +-- the second leftover is routed there anyway: +create view uv_fpo_wco_part_view as + select * from uv_fpo_wco_part where b = 0 with check option; +delete from uv_fpo_wco_part_view + for portion of valid_at from '2000-03-01' to '2000-07-01' + where lower(valid_at) < '2000-06-01'; +select tableoid::regclass, * from uv_fpo_wco_part order by valid_at; + tableoid | id | valid_at | b +------------------+-------+-------------------------+--- + uv_fpo_wco_part1 | [1,2) | [01-01-2000,03-01-2000) | 0 + uv_fpo_wco_part2 | [1,2) | [07-01-2000,01-01-2004) | 0 +(2 rows) + +-- This view rejects the leftover that lands in part2: +delete from uv_fpo_wco_part; +insert into uv_fpo_wco_part values ('[1,1]', '[2000-01-01,2004-01-01)', 0); +create view uv_fpo_wco_part_view2 as + select * from uv_fpo_wco_part where lower(valid_at) < '2000-02-01' + with check option; +delete from uv_fpo_wco_part_view2 + for portion of valid_at from '2000-03-01' to '2000-07-01' + where lower(valid_at) < '2000-06-01'; +ERROR: new row violates check option for view "uv_fpo_wco_part_view2" +DETAIL: Failing row contains ([1,2), [07-01-2000,01-01-2004), 0). +-- The base table is unchanged: +select tableoid::regclass, * from uv_fpo_wco_part order by valid_at; + tableoid | id | valid_at | b +------------------+-------+-------------------------+--- + uv_fpo_wco_part1 | [1,2) | [01-01-2000,01-01-2004) | 0 +(1 row) + +drop view uv_fpo_wco_part_view; +drop view uv_fpo_wco_part_view2; +drop table uv_fpo_wco_part; -- Test whole-row references to the view create table uv_iocu_tab (a int unique, b text); create view uv_iocu_view as diff --git a/src/test/regress/sql/for_portion_of.sql b/src/test/regress/sql/for_portion_of.sql index 955cf666d66..b61fe10478e 100644 --- a/src/test/regress/sql/for_portion_of.sql +++ b/src/test/regress/sql/for_portion_of.sql @@ -1472,6 +1472,48 @@ SELECT * FROM temporal_partitioned_5 ORDER BY id, valid_at; DROP FUNCTION fpo_returning_row; DROP TABLE temporal_partitioned; +-- Test FOR PORTION OF when the partition key depends on the range column. +-- Then a leftover can belong to a partition that is not one of the plan's +-- result relations, and we must build a new ResultRelInfo for it. + +CREATE TABLE temporal_partitioned_by_valid_at ( + id int4range, + valid_at daterange, + name text +) PARTITION BY RANGE (lower(valid_at)); +CREATE TABLE temporal_partitioned_early + PARTITION OF temporal_partitioned_by_valid_at + FOR VALUES FROM (MINVALUE) TO ('2000-06-01'); +CREATE TABLE temporal_partitioned_late + PARTITION OF temporal_partitioned_by_valid_at + FOR VALUES FROM ('2000-06-01') TO (MAXVALUE); + +INSERT INTO temporal_partitioned_by_valid_at (id, valid_at, name) VALUES + ('[1,2)', daterange('2000-01-01', '2010-01-01'), 'one'); + +-- The WHERE clause prunes away the late partition, but the second leftover +-- belongs there. The leftovers must not appear in the RETURNING output. +DELETE FROM temporal_partitioned_by_valid_at + FOR PORTION OF valid_at FROM '2000-03-01' TO '2000-07-01' + WHERE lower(valid_at) < '2000-06-01' + RETURNING id, valid_at, name; +SELECT tableoid::regclass, * FROM temporal_partitioned_by_valid_at + ORDER BY id, valid_at; + +-- The same thing for UPDATE +DELETE FROM temporal_partitioned_by_valid_at; +INSERT INTO temporal_partitioned_by_valid_at (id, valid_at, name) VALUES + ('[1,2)', daterange('2000-01-01', '2010-01-01'), 'one'); +UPDATE temporal_partitioned_by_valid_at + FOR PORTION OF valid_at FROM '2000-03-01' TO '2000-07-01' + SET name = 'one^1' + WHERE lower(valid_at) < '2000-06-01' + RETURNING id, valid_at, name; +SELECT tableoid::regclass, * FROM temporal_partitioned_by_valid_at + ORDER BY id, valid_at; + +DROP TABLE temporal_partitioned_by_valid_at; + -- UPDATE/DELETE FOR PORTION OF with RULEs CREATE TABLE fpo_rule (f1 bigint, f2 int4range); INSERT INTO fpo_rule VALUES (1, '[1, 11)'); diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql index 3ddb7b43cc8..4158bf26b74 100644 --- a/src/test/regress/sql/updatable_views.sql +++ b/src/test/regress/sql/updatable_views.sql @@ -1943,6 +1943,39 @@ drop view uv_fpo_wco_view; drop view uv_fpo_nowco_view; drop table uv_fpo_wco_tab; +-- On a partitioned table a leftover can be routed to a partition that is not +-- one of the plan's result relations. WITH CHECK OPTION must be enforced +-- there too. +create table uv_fpo_wco_part (id int4range, valid_at daterange, b int) + partition by range (lower(valid_at)); +create table uv_fpo_wco_part1 partition of uv_fpo_wco_part + for values from (minvalue) to ('2000-06-01'); +create table uv_fpo_wco_part2 partition of uv_fpo_wco_part + for values from ('2000-06-01') to (maxvalue); +insert into uv_fpo_wco_part values ('[1,1]', '[2000-01-01,2004-01-01)', 0); +-- This view accepts both leftovers. The where clause prunes away part2, but +-- the second leftover is routed there anyway: +create view uv_fpo_wco_part_view as + select * from uv_fpo_wco_part where b = 0 with check option; +delete from uv_fpo_wco_part_view + for portion of valid_at from '2000-03-01' to '2000-07-01' + where lower(valid_at) < '2000-06-01'; +select tableoid::regclass, * from uv_fpo_wco_part order by valid_at; +-- This view rejects the leftover that lands in part2: +delete from uv_fpo_wco_part; +insert into uv_fpo_wco_part values ('[1,1]', '[2000-01-01,2004-01-01)', 0); +create view uv_fpo_wco_part_view2 as + select * from uv_fpo_wco_part where lower(valid_at) < '2000-02-01' + with check option; +delete from uv_fpo_wco_part_view2 + for portion of valid_at from '2000-03-01' to '2000-07-01' + where lower(valid_at) < '2000-06-01'; +-- The base table is unchanged: +select tableoid::regclass, * from uv_fpo_wco_part order by valid_at; +drop view uv_fpo_wco_part_view; +drop view uv_fpo_wco_part_view2; +drop table uv_fpo_wco_part; + -- Test whole-row references to the view create table uv_iocu_tab (a int unique, b text); create view uv_iocu_view as -- 2.47.3