| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Paul A Jungwirth <pj(at)illuminatedcomputing(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, jian he <jian(dot)universality(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org> |
| Subject: | Re: Fix bug of UPDATE/DELETE FOR PORTION OF with inheritance tables |
| Date: | 2026-05-27 07:55:38 |
| Message-ID: | 5E0A0FF4-3E57-415E-A1C7-F1184F1EDC32@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On May 27, 2026, at 09:28, Paul A Jungwirth <pj(at)illuminatedcomputing(dot)com> wrote:
>
> On Tue, May 26, 2026 at 9:14 AM Paul A Jungwirth
> <pj(at)illuminatedcomputing(dot)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(at)illuminatedcomputing(dot)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/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Zhijie Hou (Fujitsu) | 2026-05-27 08:08:41 | RE: Adding REPACK [concurrently] |
| Previous Message | Chao Li | 2026-05-27 06:50:01 | Re: [PATCH] Improve REPACK (CONCURRENTLY) error messages for unsupported configurations |