| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> |
| Cc: | Richard Guo <guofenglinux(at)gmail(dot)com>, SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Peter Eisentraut <peter(at)eisentraut(dot)org> |
| Subject: | Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) |
| Date: | 2026-04-14 02:49:38 |
| Message-ID: | 1FE6C83B-8CAB-4643-9D97-3F8C93C6B12F@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Apr 13, 2026, at 19:03, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
>
> On Mon, 13 Apr 2026, 09:20 Richard Guo, <guofenglinux(at)gmail(dot)com> wrote:
> On Mon, Apr 13, 2026 at 4:21 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> > I think the issue is that rewriteTargetListIU() removes generated columns from the target list, as described by this comment:
>
> > Later, when the rule action is rewritten, ReplaceVarsFromTargetList() cannot find a target list entry for NEW.gen. For UPDATE rules, the missing NEW column is handled with REPLACEVARS_CHANGE_VARNO, so it falls back to referencing the original target relation row, which gives the old value.
>
> I came to the same conclusion.
>
> > One possible fix is to build a new target list that adds generated columns back when there are rules to fire. I tried the solution locally with some quick and dirty code and it seems to fix both stored and virtual generated columns for me.
>
> I think a simpler fix might be to expand generated column references
> in the NEW relation to their generation expressions before
> ReplaceVarsFromTargetList resolves NEW references, so that the base
> column Vars within the expressions can be correctly resolved.
> Something like attached.
>
> - Richard
>
> One thing about that approach is that it leads to 2 full rewrites of the rule action using ReplaceVarsFromTargetList(). I think that could be avoided by using including generated column expressions in the targetlist passed to ReplaceVarsFromTargetList() by rewriteRuleAction(). I haven't tried it, but I imagine it could reuse some code from expand_generated_columns_internal().
>
> Regards,
> Dean
I think Dean’s comment aligns with my implementation. My implementation just adds generated columns back to target list when needed, and pass the new target list to ReplaceVarsFromTargetList.
Please see the attached diff for my implementation:
* The diff is based on Richard’s v1 patch.
* As this diff is only to show my implementation rather than proposing a patch, so the diff is not well polished, and I didn’t touch the change to expand_generated_columns_internal of v1 patch.
* The v1 patch has a typo in generated_virtual.sql where “STORED” should be “VIRTUAL”, I fixed that as it impacts the tests.
* I updated the test cases by adding one more column, so that the generated column is built upon two columns.
I am really new to this area, so please don’t be surprised if I missed something or made something wrong. I am trying to learn by resolving problems.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| Attachment | Content-Type | Size |
|---|---|---|
| chao_impl.diff | application/octet-stream | 8.0 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Thomas Munro | 2026-04-14 03:04:32 | Re: ISBN range table |
| Previous Message | SATYANARAYANA NARLAPURAM | 2026-04-14 01:55:28 | Fix checkpointer PANIC due to missing fsync cancel in mdunlinkfork() |