Fix bug of UPDATE/DELETE FOR PORTION OF with inheritance tables

On May 7, 2026, at 11:40, Chao Li <li.evan.chao@gmail.com> wrote:

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/

<v1-0001-Fix-FOR-PORTION-OF-leftovers-for-inheritance-chil.patch>

Merged into [1]/messages/by-id/CAHg+QDcd=t69gLf9yQexO07EJ2mx0Z70NFHo6h94X1EDA=hM0g@mail.gmail.com.

[1]: /messages/by-id/CAHg+QDcd=t69gLf9yQexO07EJ2mx0Z70NFHo6h94X1EDA=hM0g@mail.gmail.com

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Fri, May 8, 2026 at 10:48 PM Chao Li <li.evan.chao@gmail.com> wrote:

On May 7, 2026, at 11:40, Chao Li <li.evan.chao@gmail.com> wrote:
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/

<v1-0001-Fix-FOR-PORTION-OF-leftovers-for-inheritance-chil.patch>

Merged into [1].

[1] /messages/by-id/CAHg+QDcd=t69gLf9yQexO07EJ2mx0Z70NFHo6h94X1EDA=hM0g@mail.gmail.com

Here is a new patch for this. I don't think it makes sense to combine
it with the other fix anymore.

Per [1]/messages/by-id/CA+renyXyLfvtvVv--hGWGTgzFP=-+dPLy4RWvEmioAPyJMM+uw@mail.gmail.com, we are now checking for UPDATE permission on the FOR PORTION
OF column, so most of that other patch went away, and the fix here can
be done independently. That's what v2 here does. I didn't really
change anything from the v12-0002 patch on that other thread, except
for moving some tests into the other patch (since they had nothing to
do with traditional inheritance). The patch here now passes, whether
applied to master directly or applied on top of v13 from the
updatedCols fix. (Combining them will give a rebase conflict in the
test files, but it's pretty trivial to fix.)

[1]: /messages/by-id/CA+renyXyLfvtvVv--hGWGTgzFP=-+dPLy4RWvEmioAPyJMM+uw@mail.gmail.com

Yours,

--
Paul ~{:-)
pj@illuminatedcomputing.com

On May 26, 2026, at 07:01, Paul A Jungwirth <pj@illuminatedcomputing.com> wrote:

On Fri, May 8, 2026 at 10:48 PM Chao Li <li.evan.chao@gmail.com> wrote:

On May 7, 2026, at 11:40, Chao Li <li.evan.chao@gmail.com> wrote:
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/

<v1-0001-Fix-FOR-PORTION-OF-leftovers-for-inheritance-chil.patch>

Merged into [1].

[1] /messages/by-id/CAHg+QDcd=t69gLf9yQexO07EJ2mx0Z70NFHo6h94X1EDA=hM0g@mail.gmail.com

Here is a new patch for this. I don't think it makes sense to combine
it with the other fix anymore.

Per [1], we are now checking for UPDATE permission on the FOR PORTION
OF column, so most of that other patch went away, and the fix here can
be done independently. That's what v2 here does. I didn't really
change anything from the v12-0002 patch on that other thread, except
for moving some tests into the other patch (since they had nothing to
do with traditional inheritance). The patch here now passes, whether
applied to master directly or applied on top of v13 from the
updatedCols fix. (Combining them will give a rebase conflict in the
test files, but it's pretty trivial to fix.)

[1] /messages/by-id/CA+renyXyLfvtvVv--hGWGTgzFP=-+dPLy4RWvEmioAPyJMM+uw@mail.gmail.com

Yours,

--
Paul ~{:-)
pj@illuminatedcomputing.com
<v2-0001-Fix-FOR-PORTION-OF-with-partitions-and-inheritanc.patch>

Now v2 mixes two separate changes:

1) Adding ExecInitForPortionOf() to address the UPDATE OF trigger issue
2) Fixing the inheritance/leftover bug originally reported in this thread

I’d prefer not to combine these in a single patch. Could you please split out the refactoring that adds ExecInitForPortionOf() into a separate patch, with tests showing that the UPDATE OF trigger issue is fixed? Then I can rework my inheritance/leftover fix on top of that.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Mon, May 25, 2026 at 11:55 PM Chao Li <li.evan.chao@gmail.com> wrote:

Now v2 mixes two separate changes:

1) Adding ExecInitForPortionOf() to address the UPDATE OF trigger issue
2) Fixing the inheritance/leftover bug originally reported in this thread

I’d prefer not to combine these in a single patch. Could you please split out the refactoring that adds ExecInitForPortionOf() into a separate patch, with tests showing that the UPDATE OF trigger issue is fixed? Then I can rework my inheritance/leftover fix on top of that.

Thanks for taking a look! I agree about keeping the fixes separate.
UPDATE OF triggers are fixed by the patch on the other thread (with
tests). Since we now include the application-time column in
updatedCols, there is nothing else to do there.

I thought the ExecInitForPortionOf refactoring was still nice to keep,
and it seemed to fit better here. (I should have updated the commit
message not to mention UPDATE OF though.)

But thinking about it this morning, I realized: the planner already
prunes partitions before we get here. Lazily initializing the structs
is trying too hard. Is there some scenario where that actually saves
work? If not, we could just set up all the structs in
ExecInitModifyTable. That's what I was doing in an older version of
the original patch. And then the refactor with ExecInitForPortionOf
seems unnecessary. I'll send a patch with those changes, unless you
have objections. (Probably two separate patches: one to remove lazy
initialization, another to actually fix traditional inheritance.) If
you disagree, please let me know.

Yours,

--
Paul ~{:-)
pj@illuminatedcomputing.com

On Tue, May 26, 2026 at 9:14 AM Paul A Jungwirth
<pj@illuminatedcomputing.com> wrote:

I thought the ExecInitForPortionOf refactoring was still nice to keep,
and it seemed to fit better here. (I should have updated the commit
message not to mention UPDATE OF though.)

I looked at the original v1 patch again. I thought the commit message
was excellent, so I pulled that into the latest patch, also some
comment changes and the partitionRouting boolean and some test
double-checks. I still like how ExecInitForPortionOf cuts down on the
branchiness and the variables used to track attnums, and also how it
gathers a lot of the child table setup in one place. The v1 seemed
hard to trace all the cases from top to bottom. For instance it
initialized map for all child tables, but then only used it for
partitions.

But thinking about it this morning, I realized: the planner already
prunes partitions before we get here. Lazily initializing the structs
is trying too hard. Is there some scenario where that actually saves
work?

Never mind, of course you can filter rows (and whole tables) besides
just by partition pruning/constraint exclusion.

I'm not sure whom to list as author/co-author/reviewer for this patch,
but I took a stab at it. I think most of the current code is from jian
he's version, but all three of us have contributed a lot by this
point.

Yours,

--
Paul ~{:-)
pj@illuminatedcomputing.com

On May 27, 2026, at 09:28, Paul A Jungwirth <pj@illuminatedcomputing.com> wrote:

On Tue, May 26, 2026 at 9:14 AM Paul A Jungwirth
<pj@illuminatedcomputing.com> wrote:

I thought the ExecInitForPortionOf refactoring was still nice to keep,
and it seemed to fit better here. (I should have updated the commit
message not to mention UPDATE OF though.)

I looked at the original v1 patch again. I thought the commit message
was excellent, so I pulled that into the latest patch, also some
comment changes and the partitionRouting boolean and some test
double-checks. I still like how ExecInitForPortionOf cuts down on the
branchiness and the variables used to track attnums, and also how it
gathers a lot of the child table setup in one place. The v1 seemed
hard to trace all the cases from top to bottom. For instance it
initialized map for all child tables, but then only used it for
partitions.

But thinking about it this morning, I realized: the planner already
prunes partitions before we get here. Lazily initializing the structs
is trying too hard. Is there some scenario where that actually saves
work?

Never mind, of course you can filter rows (and whole tables) besides
just by partition pruning/constraint exclusion.

I'm not sure whom to list as author/co-author/reviewer for this patch,
but I took a stab at it. I think most of the current code is from jian
he's version, but all three of us have contributed a lot by this
point.

Yours,

--
Paul ~{:-)
pj@illuminatedcomputing.com

Hi Paul,

I just tested v3 with my original repro, and it has resolved the bug I reported.
```
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
----+----------+------
(0 rows)
evantest=# select * from c;
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)
```

I also noticed that the “UPDATE OF” related information has been removed from the commit message. I can still reproduce the UPDATE OF issue with v3, but I think that is expected, and that issue will be resolved by the other patch.

So v3 looks clean and good to me. Thanks for updating the patch and making the two patches decoupled.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On May 27, 2026, at 15:55, Chao Li <li.evan.chao@gmail.com> wrote:

On May 27, 2026, at 09:28, Paul A Jungwirth <pj@illuminatedcomputing.com> wrote:

On Tue, May 26, 2026 at 9:14 AM Paul A Jungwirth
<pj@illuminatedcomputing.com> wrote:

I thought the ExecInitForPortionOf refactoring was still nice to keep,
and it seemed to fit better here. (I should have updated the commit
message not to mention UPDATE OF though.)

I looked at the original v1 patch again. I thought the commit message
was excellent, so I pulled that into the latest patch, also some
comment changes and the partitionRouting boolean and some test
double-checks. I still like how ExecInitForPortionOf cuts down on the
branchiness and the variables used to track attnums, and also how it
gathers a lot of the child table setup in one place. The v1 seemed
hard to trace all the cases from top to bottom. For instance it
initialized map for all child tables, but then only used it for
partitions.

But thinking about it this morning, I realized: the planner already
prunes partitions before we get here. Lazily initializing the structs
is trying too hard. Is there some scenario where that actually saves
work?

Never mind, of course you can filter rows (and whole tables) besides
just by partition pruning/constraint exclusion.

I'm not sure whom to list as author/co-author/reviewer for this patch,
but I took a stab at it. I think most of the current code is from jian
he's version, but all three of us have contributed a lot by this
point.

Yours,

--
Paul ~{:-)
pj@illuminatedcomputing.com

Hi Paul,

I just tested v3 with my original repro, and it has resolved the bug I reported.
```
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
----+----------+------
(0 rows)
evantest=# select * from c;
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)
```

I also noticed that the “UPDATE OF” related information has been removed from the commit message. I can still reproduce the UPDATE OF issue with v3, but I think that is expected, and that issue will be resolved by the other patch.

So v3 looks clean and good to me. Thanks for updating the patch and making the two patches decoupled.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Rebased as requested by the CF.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On 07.06.26 05:03, Chao Li wrote:

I also noticed that the “UPDATE OF” related information has been removed from the commit message. I can still reproduce the UPDATE OF issue with v3, but I think that is expected, and that issue will be resolved by the other patch.

So v3 looks clean and good to me. Thanks for updating the patch and making the two patches decoupled.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Rebased as requested by the CF.

committed