| From: | Paul A Jungwirth <pj(at)illuminatedcomputing(dot)com> |
|---|---|
| To: | SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com> |
| Cc: | jian he <jian(dot)universality(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: FOR PORTION OF does not recompute GENERATED STORED columns that depend on the range column |
| Date: | 2026-04-15 20:59:21 |
| Message-ID: | CA+renyWD+XXifwswE74vhjooqbiVKu4qVhLvpMcUQBzrjVjT7A@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Fri, Apr 10, 2026 at 3:01 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram(at)gmail(dot)com> wrote:
>
>> I've combined all these changes into a single patch for now, as they
>> seem closely related.
>>
>> [1]: https://postgr.es/m/CACJufxHALFKca5SMn5DNnbrX2trPamVL6napn_nm35p15yw+rg@mail.gmail.com
>
> I applied your patch and tested. The following scenarios are now passing: (1) table inheritance issue I reported in [1], (2) issue reported in this thread.
They look good to me too. I read through the patch and made some
stylistic changes, as well as some grammar/typo fixes. In
ExecInitForPortionOf I tried to group things for each case more
closely, instead of separate disconnected branches. I also added/moved
some comments. Please see the attached.
There's something I think we could still improve: we omit the
valid-time in updatedCols, since that bitmapset is for permissions
checking (at least originally), but now other features are using it as
well. Our fix adds special logic to consider the valid-at column for
GENERATED column dependencies and more special logic for UPDATE OF
triggers. Perhaps we should add the attno to updatedCols after all,
and put the special logic in the permissions check instead? That seems
simpler and more robust. Or maybe it's time to have separate
bitmapsets, one for permissions and another for everything else? What
do you think?
I'm not sure we should be consolidating these fixes into one patch.
But I tried to call out all the issues in my commit message.
> Following are still failing:
>
> (1) instead of triggers + views, mentioned in the thread [2], it has both the test case and the fix.
I'll follow up there.
> (2) For Portion Of DELETE loses rows when a BEFORE INSERT trigger returns NULL
>
> ...
>
> (3) FPO UPDATE loses leftovers the same way
I agree with jian that this is the correct behavior. The inserts are
supposed to fire triggers, and if a trigger signals to cancel the
insert, that's what we should do.
Yours,
--
Paul ~{:-)
pj(at)illuminatedcomputing(dot)com
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-Fix-some-problems-with-UPDATE-FOR-PORTION-OF.patch | application/octet-stream | 22.3 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-04-15 21:25:24 | Re: Reduce build times of pg_trgm GIN indexes |
| Previous Message | Bruce Momjian | 2026-04-15 20:51:16 | Re: First draft of PG 19 release notes |