public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Subject: Fix bug of UPDATE/DELETE FOR PORTION OF with inheritance tables
Date: Thu, 7 May 2026 11:40:12 +0800
Message-ID: <[email protected]> (raw)
Hi,
While testing UPDATE FOR PORTION OF, I found a bug with inheritance tables. The following repro shows the problem more clearly than a description in words:
```
evantest=# create table p (id int, valid_at daterange, name text);
CREATE TABLE
evantest=# create table c (extra text) inherits (p);
CREATE TABLE
evantest=# insert into c values (1, daterange('2000-01-01', '2010-01-01'), 'old', 'x');
INSERT 0 1
evantest=# update p for portion of valid_at from '2001-01-01' to '2002-01-01' set name = 'new' where id = 1;
UPDATE 1
evantest=# select * from only p;
id | valid_at | name
----+-------------------------+------
1 | [2000-01-01,2001-01-01) | old
1 | [2002-01-01,2010-01-01) | old
(2 rows)
evantest=# select * from only c;
id | valid_at | name | extra
----+-------------------------+------+-------
1 | [2001-01-01,2002-01-01) | new | x
(1 row)
```
In this repro, the original tuple is inserted into the child table c, while the parent table p is empty. After the update, the updated portion is left in c, but the two leftover ranges are inserted into p, which is clearly wrong.
The same bug exists for DELETE FOR PORTION OF with inheritance tables as well:
```
evantest=# delete from p for portion of valid_at from '2001-01-01' to '2002-01-01' where id = 1;
DELETE 1
evantest=# select * from only p;
id | valid_at | name
----+-------------------------+------
1 | [2000-01-01,2001-01-01) | old
1 | [2002-01-01,2010-01-01) | old
(2 rows)
evantest=# select * from only c;
id | valid_at | name | extra
----+----------+------+-------
(0 rows)
```
After looking into the code, I found that leftover row insertion only considers the partitioned-table case, where leftovers need to be inserted through the root relation for partition routing. Plain inheritance is different, leftover rows should be inserted back into the actual child relation.
While debugging this, I also noticed another issue around mapping the range column’s attnum. In multiple-inheritance cases, the range column’s attnum in a child table may be different from the one in its parent, so we need to use the child’s actual attnum.
Please see the attached patch for the fix details and the new tests. Since I believe this bug was introduced in 19, I’m going to add it to the open items.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v1-0001-Fix-FOR-PORTION-OF-leftovers-for-inheritance-chil.patch (14.1K, 2-v1-0001-Fix-FOR-PORTION-OF-leftovers-for-inheritance-chil.patch)
download | inline diff:
From 92abeba40fb1ef795a7f0eea198d7c144aef20fe Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Thu, 7 May 2026 11:23:10 +0800
Subject: [PATCH v1] Fix FOR PORTION OF leftovers for inheritance children
ExecForPortionOfLeftovers() assumed that any result relation with
ri_RootResultRelInfo should reinsert temporal leftovers through the root
relation. That is correct for partitioned tables, where tuple routing is
needed, but it is wrong for plain inheritance.
When UPDATE/DELETE FOR PORTION OF is run on an inheritance parent and a
child row is split, the leftover rows must be inserted back into the child
relation. Reinserting through the parent can lose child-only columns and
place the leftover rows in the wrong relation.
Fix this by distinguishing partitioned-table routing from plain
inheritance. For partitioned tables, keep using the root leftover slot and
insert through the root relation. For plain inheritance children, use a
leftover slot matching the child relation and insert directly into the
child. Also keep translating the application-time column attno for child
relations, so multiple-inheritance cases with different attribute numbers
are handled correctly.
Add regression tests for UPDATE and DELETE FOR PORTION OF on inheritance
children, including a multiple-inheritance case where the range column has
a different attnum in the parent and child.
Author: Chao Li <[email protected]>
Reviewed-by:
Discussion: https://postgr.es/m/
---
src/backend/executor/nodeModifyTable.c | 76 ++++++++----
src/test/regress/expected/for_portion_of.out | 119 +++++++++++++++++++
src/test/regress/sql/for_portion_of.sql | 80 +++++++++++++
3 files changed, 252 insertions(+), 23 deletions(-)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 4cb057ca4f9..df3e3bb1b98 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1409,7 +1409,11 @@ ExecForPortionOfLeftovers(ModifyTableContext *context,
ModifyTableState *mtstate = context->mtstate;
ModifyTable *node = (ModifyTable *) mtstate->ps.plan;
ForPortionOfExpr *forPortionOf = (ForPortionOfExpr *) node->forPortionOf;
- AttrNumber rangeAttno;
+ AttrNumber actualRangeAttno; /* attno of range column in the current
+ * result relation */
+ AttrNumber targetRangeAttno; /* attno of range column in the target
+ * table of the query */
+ AttrNumber leftoverRangeAttno;
Datum oldRange;
TypeCacheEntry *typcache;
ForPortionOfState *fpoState;
@@ -1422,18 +1426,30 @@ ExecForPortionOfLeftovers(ModifyTableContext *context,
FmgrInfo flinfo;
PgStat_FunctionCallUsage fcusage;
ReturnSetInfo rsi;
+ ResultRelInfo *rootRri = resultRelInfo->ri_RootResultRelInfo;
bool didInit = false;
bool shouldFree = false;
+ bool partitionRouting;
LOCAL_FCINFO(fcinfo, 2);
+ partitionRouting =
+ rootRri != NULL &&
+ rootRri->ri_RelationDesc->rd_rel->relkind == RELKIND_PARTITIONED_TABLE;
+ targetRangeAttno = forPortionOf->rangeVar->varattno;
+
if (!resultRelInfo->ri_forPortionOf)
{
/*
- * If we don't have a ForPortionOfState yet, we must be a partition
- * child being hit for the first time. Make a copy from the root, with
- * our own TupleTableSlot. We do this lazily so that we don't pay the
- * price of unused partitions.
+ * If we don't have a ForPortionOfState yet, we must be a child
+ * relation being hit for the first time. Make a copy from the root,
+ * with slots matching the child's tuple descriptor.
+ *
+ * Partitions share the root leftover slot, since they must insert via
+ * the root relation to get tuple routing. Plain inheritance children
+ * must keep their own leftover slot and insert back into the child,
+ * or else child-only column values and physical placement would be
+ * lost.
*/
ForPortionOfState *leafState = makeNode(ForPortionOfState);
@@ -1447,8 +1463,15 @@ ExecForPortionOfLeftovers(ModifyTableContext *context,
leafState->fp_rangeType = fpoState->fp_rangeType;
leafState->fp_rangeAttno = fpoState->fp_rangeAttno;
leafState->fp_targetRange = fpoState->fp_targetRange;
- leafState->fp_Leftover = fpoState->fp_Leftover;
- /* Each partition needs a slot matching its tuple descriptor */
+ if (partitionRouting)
+ leafState->fp_Leftover = fpoState->fp_Leftover;
+ else
+ leafState->fp_Leftover =
+ ExecInitExtraTupleSlot(mtstate->ps.state,
+ resultRelInfo->ri_RelationDesc->rd_att,
+ &TTSOpsVirtual);
+
+ /* Each child relation needs a slot matching its tuple descriptor */
leafState->fp_Existing =
table_slot_create(resultRelInfo->ri_RelationDesc,
&mtstate->ps.state->es_tupleTable);
@@ -1476,20 +1499,22 @@ ExecForPortionOfLeftovers(ModifyTableContext *context,
elog(ERROR, "failed to fetch tuple for FOR PORTION OF");
/*
- * Get the old range of the record being updated/deleted. Must read with
- * the attno of the leaf partition being updated.
+ * Get the old range of the record being updated/deleted. The query
+ * expression uses the target relation's attno, but the tuple slot may
+ * belong to a child relation. If so, translate it to the corresponding
+ * child attno.
*/
- rangeAttno = forPortionOf->rangeVar->varattno;
- if (resultRelInfo->ri_RootResultRelInfo)
+ actualRangeAttno = targetRangeAttno;
+ if (rootRri != NULL)
map = ExecGetChildToRootMap(resultRelInfo);
if (map != NULL)
- rangeAttno = map->attrMap->attnums[rangeAttno - 1];
+ actualRangeAttno = map->attrMap->attnums[actualRangeAttno - 1];
slot_getallattrs(oldtupleSlot);
- if (oldtupleSlot->tts_isnull[rangeAttno - 1])
+ if (oldtupleSlot->tts_isnull[actualRangeAttno - 1])
elog(ERROR, "found a NULL range in a temporal table");
- oldRange = oldtupleSlot->tts_values[rangeAttno - 1];
+ oldRange = oldtupleSlot->tts_values[actualRangeAttno - 1];
/*
* Get the range's type cache entry. This is worth caching for the whole
@@ -1529,10 +1554,15 @@ ExecForPortionOfLeftovers(ModifyTableContext *context,
/*
* If there are partitions, we must insert into the root table, so we get
* tuple routing. We already set up leftoverSlot with the root tuple
- * descriptor.
+ * descriptor for that case.
*/
- if (resultRelInfo->ri_RootResultRelInfo)
- resultRelInfo = resultRelInfo->ri_RootResultRelInfo;
+ if (partitionRouting)
+ {
+ resultRelInfo = rootRri;
+ leftoverRangeAttno = targetRangeAttno;
+ }
+ else
+ leftoverRangeAttno = actualRangeAttno;
/*
* Insert a leftover for each value returned by the without_portion helper
@@ -1574,11 +1604,11 @@ ExecForPortionOfLeftovers(ModifyTableContext *context,
{
/*
* Make a copy of the pre-UPDATE row. Then we'll overwrite the
- * range column below. Convert oldtuple to the base table's format
- * if necessary. We need to insert temporal leftovers through the
- * root partition so they get routed correctly.
+ * range column below. Only partitioned targets need conversion to
+ * the root table's format, because they reinsert through the root
+ * relation for tuple routing.
*/
- if (map != NULL)
+ if (partitionRouting && map != NULL)
{
leftoverSlot = execute_attr_map_slot(map->attrMap,
oldtupleSlot,
@@ -1601,8 +1631,8 @@ ExecForPortionOfLeftovers(ModifyTableContext *context,
didInit = true;
}
- leftoverSlot->tts_values[forPortionOf->rangeVar->varattno - 1] = leftover;
- leftoverSlot->tts_isnull[forPortionOf->rangeVar->varattno - 1] = false;
+ leftoverSlot->tts_values[leftoverRangeAttno - 1] = leftover;
+ leftoverSlot->tts_isnull[leftoverRangeAttno - 1] = false;
ExecMaterializeSlot(leftoverSlot);
/*
diff --git a/src/test/regress/expected/for_portion_of.out b/src/test/regress/expected/for_portion_of.out
index 0c0a205c44b..7d7847c9ad7 100644
--- a/src/test/regress/expected/for_portion_of.out
+++ b/src/test/regress/expected/for_portion_of.out
@@ -2097,6 +2097,125 @@ SELECT * FROM temporal_partitioned_5 ORDER BY id, valid_at;
(4 rows)
DROP TABLE temporal_partitioned;
+-- UPDATE/DELETE FOR PORTION OF with inheritance
+CREATE TABLE fpo_parent (
+ id int,
+ valid_at daterange,
+ name text
+);
+CREATE TABLE fpo_child (
+ extra text
+) INHERITS (fpo_parent);
+INSERT INTO fpo_child VALUES
+ (1, daterange('2000-01-01', '2010-01-01'), 'old', 'x');
+UPDATE fpo_parent FOR PORTION OF valid_at FROM '2001-01-01' TO '2002-01-01'
+ SET name = 'new'
+ WHERE id = 1;
+SELECT * FROM ONLY fpo_parent;
+ id | valid_at | name
+----+----------+------
+(0 rows)
+
+SELECT * FROM ONLY fpo_child;
+ id | valid_at | name | extra
+----+-------------------------+------+-------
+ 1 | [2001-01-01,2002-01-01) | new | x
+ 1 | [2000-01-01,2001-01-01) | old | x
+ 1 | [2002-01-01,2010-01-01) | old | x
+(3 rows)
+
+TRUNCATE TABLE fpo_child, fpo_parent;
+INSERT INTO fpo_child VALUES
+ (1, daterange('2000-01-01', '2010-01-01'), 'old', 'x');
+DELETE FROM fpo_parent FOR PORTION OF valid_at FROM '2001-01-01' TO '2002-01-01'
+ WHERE id = 1;
+SELECT * FROM ONLY fpo_parent;
+ id | valid_at | name
+----+----------+------
+(0 rows)
+
+SELECT * FROM ONLY fpo_child;
+ id | valid_at | name | extra
+----+-------------------------+------+-------
+ 1 | [2000-01-01,2001-01-01) | old | x
+ 1 | [2002-01-01,2010-01-01) | old | x
+(2 rows)
+
+DROP TABLE fpo_child, fpo_parent;
+-- UPDATE/DELETE FOR PORTION OF with inheritance and differing attnos
+CREATE TABLE temporal_parent (
+ id int,
+ valid_at daterange,
+ name text
+);
+CREATE TABLE other_parent (
+ prefix text,
+ note text
+);
+CREATE TABLE mi_child () INHERITS (other_parent, temporal_parent);
+-- attnum of the range column is different in temporal_parent and mi_child
+SELECT attnum, attname
+ FROM pg_attribute
+ WHERE attrelid = 'temporal_parent'::regclass
+ AND attnum > 0 AND NOT attisdropped
+ ORDER BY attnum;
+ attnum | attname
+--------+----------
+ 1 | id
+ 2 | valid_at
+ 3 | name
+(3 rows)
+
+SELECT attnum, attname
+ FROM pg_attribute
+ WHERE attrelid = 'mi_child'::regclass
+ AND attnum > 0 AND NOT attisdropped
+ ORDER BY attnum;
+ attnum | attname
+--------+----------
+ 1 | prefix
+ 2 | note
+ 3 | id
+ 4 | valid_at
+ 5 | name
+(5 rows)
+
+INSERT INTO mi_child VALUES
+ ('pfx', 'memo', 1, daterange('2000-01-01', '2010-01-01'), 'old');
+UPDATE temporal_parent FOR PORTION OF valid_at FROM '2001-01-01' TO '2002-01-01'
+ SET name = 'new'
+ WHERE id = 1;
+SELECT * FROM ONLY temporal_parent;
+ id | valid_at | name
+----+----------+------
+(0 rows)
+
+SELECT * FROM ONLY mi_child;
+ prefix | note | id | valid_at | name
+--------+------+----+-------------------------+------
+ pfx | memo | 1 | [2001-01-01,2002-01-01) | new
+ pfx | memo | 1 | [2000-01-01,2001-01-01) | old
+ pfx | memo | 1 | [2002-01-01,2010-01-01) | old
+(3 rows)
+
+TRUNCATE TABLE mi_child, other_parent, temporal_parent;
+INSERT INTO mi_child VALUES
+ ('pfx', 'memo', 1, daterange('2000-01-01', '2010-01-01'), 'old');
+DELETE FROM temporal_parent FOR PORTION OF valid_at FROM '2001-01-01' TO '2002-01-01'
+ WHERE id = 1;
+SELECT * FROM ONLY temporal_parent;
+ id | valid_at | name
+----+----------+------
+(0 rows)
+
+SELECT * FROM ONLY mi_child;
+ prefix | note | id | valid_at | name
+--------+------+----+-------------------------+------
+ pfx | memo | 1 | [2000-01-01,2001-01-01) | old
+ pfx | memo | 1 | [2002-01-01,2010-01-01) | old
+(2 rows)
+
+DROP TABLE mi_child, other_parent, temporal_parent;
-- 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/for_portion_of.sql b/src/test/regress/sql/for_portion_of.sql
index fd79a9b78e7..83a2bc9a82c 100644
--- a/src/test/regress/sql/for_portion_of.sql
+++ b/src/test/regress/sql/for_portion_of.sql
@@ -1365,6 +1365,86 @@ SELECT * FROM temporal_partitioned_5 ORDER BY id, valid_at;
DROP TABLE temporal_partitioned;
+-- UPDATE/DELETE FOR PORTION OF with inheritance
+CREATE TABLE fpo_parent (
+ id int,
+ valid_at daterange,
+ name text
+);
+CREATE TABLE fpo_child (
+ extra text
+) INHERITS (fpo_parent);
+
+INSERT INTO fpo_child VALUES
+ (1, daterange('2000-01-01', '2010-01-01'), 'old', 'x');
+
+UPDATE fpo_parent FOR PORTION OF valid_at FROM '2001-01-01' TO '2002-01-01'
+ SET name = 'new'
+ WHERE id = 1;
+
+SELECT * FROM ONLY fpo_parent;
+SELECT * FROM ONLY fpo_child;
+
+TRUNCATE TABLE fpo_child, fpo_parent;
+
+INSERT INTO fpo_child VALUES
+ (1, daterange('2000-01-01', '2010-01-01'), 'old', 'x');
+
+DELETE FROM fpo_parent FOR PORTION OF valid_at FROM '2001-01-01' TO '2002-01-01'
+ WHERE id = 1;
+
+SELECT * FROM ONLY fpo_parent;
+SELECT * FROM ONLY fpo_child;
+
+DROP TABLE fpo_child, fpo_parent;
+
+-- UPDATE/DELETE FOR PORTION OF with inheritance and differing attnos
+CREATE TABLE temporal_parent (
+ id int,
+ valid_at daterange,
+ name text
+);
+CREATE TABLE other_parent (
+ prefix text,
+ note text
+);
+CREATE TABLE mi_child () INHERITS (other_parent, temporal_parent);
+
+-- attnum of the range column is different in temporal_parent and mi_child
+SELECT attnum, attname
+ FROM pg_attribute
+ WHERE attrelid = 'temporal_parent'::regclass
+ AND attnum > 0 AND NOT attisdropped
+ ORDER BY attnum;
+SELECT attnum, attname
+ FROM pg_attribute
+ WHERE attrelid = 'mi_child'::regclass
+ AND attnum > 0 AND NOT attisdropped
+ ORDER BY attnum;
+
+INSERT INTO mi_child VALUES
+ ('pfx', 'memo', 1, daterange('2000-01-01', '2010-01-01'), 'old');
+
+UPDATE temporal_parent FOR PORTION OF valid_at FROM '2001-01-01' TO '2002-01-01'
+ SET name = 'new'
+ WHERE id = 1;
+
+SELECT * FROM ONLY temporal_parent;
+SELECT * FROM ONLY mi_child;
+
+TRUNCATE TABLE mi_child, other_parent, temporal_parent;
+
+INSERT INTO mi_child VALUES
+ ('pfx', 'memo', 1, daterange('2000-01-01', '2010-01-01'), 'old');
+
+DELETE FROM temporal_parent FOR PORTION OF valid_at FROM '2001-01-01' TO '2002-01-01'
+ WHERE id = 1;
+
+SELECT * FROM ONLY temporal_parent;
+SELECT * FROM ONLY mi_child;
+
+DROP TABLE mi_child, other_parent, temporal_parent;
+
-- UPDATE/DELETE FOR PORTION OF with RULEs
CREATE TABLE fpo_rule (f1 bigint, f2 int4range);
INSERT INTO fpo_rule VALUES (1, '[1, 11)');
--
2.50.1 (Apple Git-155)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: Fix bug of UPDATE/DELETE FOR PORTION OF with inheritance tables
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox