Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Fix incorrect NEW references to generated columns in rule rewriting

  1. Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)

    SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com> — 2026-04-13T01:59:28Z

    Hi hackers,
    
    NEW.<generated_coulmn> is resolved to the OLD row's value
    for update or NULL for insert cases in a DO ALSO rule action for
    generated columns.  This bug affects both stored and virtual
    generated columns. Reporting here to see if this is a known issue
    with generated columns.
    
    Repro:
    
    CREATE TABLE t (id int PRIMARY KEY, a int,
                    gen int GENERATED ALWAYS AS (a * 2) VIRTUAL);
    CREATE TABLE t_log (op text, old_gen int, new_gen int);
    
    CREATE RULE t_log AS ON UPDATE TO t
      DO ALSO INSERT INTO t_log VALUES ('UPD', OLD.gen, NEW.gen);
    
    INSERT INTO t (id, a) VALUES (1, 5);
    UPDATE t SET a = 100 WHERE id = 1;
    
    postgres=# SELECT * FROM t;
     id |  a  | gen
    ----+-----+-----
      1 | 100 | 200
    (1 row)
    
    postgres=# SELECT * FROM t_log;
     op  | old_gen | new_gen
    -----+---------+---------
     UPD |      10 |      10
    (1 row)
    
    Thanks,
    Satya
    
  2. Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)

    Richard Guo <guofenglinux@gmail.com> — 2026-04-13T02:35:52Z

    On Mon, Apr 13, 2026 at 10:59 AM SATYANARAYANA NARLAPURAM
    <satyanarlapuram@gmail.com> wrote:
    > NEW.<generated_coulmn> is resolved to the OLD row's value
    > for update or NULL for insert cases in a DO ALSO rule action for
    > generated columns.  This bug affects both stored and virtual
    > generated columns. Reporting here to see if this is a known issue
    > with generated columns.
    
    I didn't find related item in open items.  This does not seem to be a
    known issue.  I think we should fix it anyway.
    
    cc-ing Peter.
    
    - Richard
    
    
    
    
  3. Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)

    Chao Li <li.evan.chao@gmail.com> — 2026-04-13T07:21:05Z

    
    > On Apr 13, 2026, at 10:35, Richard Guo <guofenglinux@gmail.com> wrote:
    > 
    > On Mon, Apr 13, 2026 at 10:59 AM SATYANARAYANA NARLAPURAM
    > <satyanarlapuram@gmail.com> wrote:
    >> NEW.<generated_coulmn> is resolved to the OLD row's value
    >> for update or NULL for insert cases in a DO ALSO rule action for
    >> generated columns.  This bug affects both stored and virtual
    >> generated columns. Reporting here to see if this is a known issue
    >> with generated columns.
    > 
    > I didn't find related item in open items.  This does not seem to be a
    > known issue.  I think we should fix it anyway.
    > 
    > cc-ing Peter.
    > 
    > - Richard
    > 
    
    Hi Richard and Satya,
    
    I reproduced the bug following Satya’s procedure and spent some time debugging it.
    
    I think the issue is that rewriteTargetListIU() removes generated columns from the target list, as described by this comment:
    ```
            if (att_tup->attgenerated)
            {
                /*
                 * virtual generated column stores a null value; stored generated
                 * column will be fixed in executor
                 */
                new_tle = NULL;
            }
    ```
    
    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.
    
    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.
    
    Do either of you plan to propose a patch for this? If so, please go ahead and I can review it. Otherwise, I can propose a patch in a couple of days.
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
    
    
    
  4. Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)

    SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com> — 2026-04-13T07:27:44Z

    Hi Chao,
    
    On Mon, Apr 13, 2026 at 12:21 AM Chao Li <li.evan.chao@gmail.com> wrote:
    
    >
    >
    > > On Apr 13, 2026, at 10:35, Richard Guo <guofenglinux@gmail.com> wrote:
    > >
    > > On Mon, Apr 13, 2026 at 10:59 AM SATYANARAYANA NARLAPURAM
    > > <satyanarlapuram@gmail.com> wrote:
    > >> NEW.<generated_coulmn> is resolved to the OLD row's value
    > >> for update or NULL for insert cases in a DO ALSO rule action for
    > >> generated columns.  This bug affects both stored and virtual
    > >> generated columns. Reporting here to see if this is a known issue
    > >> with generated columns.
    > >
    > > I didn't find related item in open items.  This does not seem to be a
    > > known issue.  I think we should fix it anyway.
    > >
    > > cc-ing Peter.
    > >
    > > - Richard
    > >
    >
    > Hi Richard and Satya,
    >
    > I reproduced the bug following Satya’s procedure and spent some time
    > debugging it.
    >
    > I think the issue is that rewriteTargetListIU() removes generated columns
    > from the target list, as described by this comment:
    > ```
    >         if (att_tup->attgenerated)
    >         {
    >             /*
    >              * virtual generated column stores a null value; stored
    > generated
    >              * column will be fixed in executor
    >              */
    >             new_tle = NULL;
    >         }
    > ```
    >
    > 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.
    >
    > 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.
    >
    > Do either of you plan to propose a patch for this? If so, please go ahead
    > and I can review it. Otherwise, I can propose a patch in a couple of days.
    
    
    I have a patch with me, let me post it shortly.
    
    Thanks,
    Satya
    
  5. Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)

    Richard Guo <guofenglinux@gmail.com> — 2026-04-13T08:20:35Z

    On Mon, Apr 13, 2026 at 4:21 PM Chao Li <li.evan.chao@gmail.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
    
  6. Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)

    Richard Guo <guofenglinux@gmail.com> — 2026-04-13T08:30:39Z

    On Mon, Apr 13, 2026 at 4:27 PM SATYANARAYANA NARLAPURAM
    <satyanarlapuram@gmail.com> wrote:
    > On Mon, Apr 13, 2026 at 12:21 AM Chao Li <li.evan.chao@gmail.com> wrote:
    >> Do either of you plan to propose a patch for this? If so, please go ahead and I can review it. Otherwise, I can propose a patch in a couple of days.
    
    > I have a patch with me, let me post it shortly.
    
    Please feel free to share your patches.  I've been looking into this
    as well, but I'd be happy to see independent takes on it.
    
    - Richard
    
    
    
    
  7. Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)

    SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com> — 2026-04-13T08:42:49Z

    Hi Richard,
    
    On Mon, Apr 13, 2026 at 1:30 AM Richard Guo <guofenglinux@gmail.com> wrote:
    
    > On Mon, Apr 13, 2026 at 4:27 PM SATYANARAYANA NARLAPURAM
    > <satyanarlapuram@gmail.com> wrote:
    > > On Mon, Apr 13, 2026 at 12:21 AM Chao Li <li.evan.chao@gmail.com> wrote:
    > >> Do either of you plan to propose a patch for this? If so, please go
    > ahead and I can review it. Otherwise, I can propose a patch in a couple of
    > days.
    >
    > > I have a patch with me, let me post it shortly.
    >
    > Please feel free to share your patches.  I've been looking into this
    > as well, but I'd be happy to see independent takes on it.
    >
    
    I reviewed your patch, it looks good. I ran the tests you added and a few
    additional tests and they are passing. Please continue with your patch.
    Maybe a test like "WHERE NEW.b > threshold" to confirm the expansion
    happens before qual evaluation would be good.
    
    Thanks,
    Satya
    
  8. Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2026-04-13T11:03:53Z

    On Mon, 13 Apr 2026, 09:20 Richard Guo, <guofenglinux@gmail.com> wrote:
    
    > On Mon, Apr 13, 2026 at 4:21 PM Chao Li <li.evan.chao@gmail.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
    
    >
    
  9. Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)

    Chao Li <li.evan.chao@gmail.com> — 2026-04-14T02:49:38Z

    
    > On Apr 13, 2026, at 19:03, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
    > 
    > On Mon, 13 Apr 2026, 09:20 Richard Guo, <guofenglinux@gmail.com> wrote:
    > On Mon, Apr 13, 2026 at 4:21 PM Chao Li <li.evan.chao@gmail.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/
    
    
    
    
    
  10. Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)

    Richard Guo <guofenglinux@gmail.com> — 2026-04-14T03:27:39Z

    On Mon, Apr 13, 2026 at 8:04 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
    > On Mon, 13 Apr 2026, 09:20 Richard Guo, <guofenglinux@gmail.com> wrote:
    >> 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.
    
    > 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().
    
    I considered it, but I'm afraid it doesn't work directly, because
    replace_rte_variables_mutator returns the callback's replacement node
    without recursing into its children.
    
    Take Satya's repro as an example.  If we add the generation expression
    for gen to the UPDATE's targetlist, the list would be:
    
        TargetEntry 1: resno=2, expr=Const(100)       -- a = 100
        TargetEntry 2: resno=3, expr=Var(3, 2) * 2    -- gen = NEW.a * 2
    
    When ReplaceVarsFromTargetList processes Var(3, 3) (NEW.gen) in the
    rule action, it finds resno=3 and substitutes Var(3, 2) * 2.  However,
    replace_rte_variables_mutator returns this replacement directly to its
    caller; it does not recurse into the replacement's children to look
    for further matching Vars.  So the inner Var(3, 2) (NEW.a) is never
    processed, even though resno=2 with Const(100) is right there in the
    targetlist.  The Var(3, 2) survives into the planner and would cause
    problems.
    
    It could be made to work by pre-resolving the generation expressions'
    base column Vars before adding them to the UPDATE's targetlist.  For
    each generated column, we'd call ReplaceVarsFromTargetList on the
    generation expression to resolve its base column Vars, then add the
    fully resolved expression to the targetlist.  But this seems to add
    code complexity.  And I'm not sure about the performance difference
    between these two approaches.  I expect that rule action trees are
    typically small.
    
    - Richard
    
    
    
    
  11. Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)

    Chao Li <li.evan.chao@gmail.com> — 2026-04-14T06:28:24Z

    
    > On Apr 14, 2026, at 11:27, Richard Guo <guofenglinux@gmail.com> wrote:
    > 
    > On Mon, Apr 13, 2026 at 8:04 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
    >> On Mon, 13 Apr 2026, 09:20 Richard Guo, <guofenglinux@gmail.com> wrote:
    >>> 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.
    > 
    >> 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().
    > 
    > I considered it, but I'm afraid it doesn't work directly, because
    > replace_rte_variables_mutator returns the callback's replacement node
    > without recursing into its children.
    > 
    > Take Satya's repro as an example.  If we add the generation expression
    > for gen to the UPDATE's targetlist, the list would be:
    > 
    >    TargetEntry 1: resno=2, expr=Const(100)       -- a = 100
    >    TargetEntry 2: resno=3, expr=Var(3, 2) * 2    -- gen = NEW.a * 2
    > 
    > When ReplaceVarsFromTargetList processes Var(3, 3) (NEW.gen) in the
    > rule action, it finds resno=3 and substitutes Var(3, 2) * 2.  However,
    > replace_rte_variables_mutator returns this replacement directly to its
    > caller; it does not recurse into the replacement's children to look
    > for further matching Vars.  So the inner Var(3, 2) (NEW.a) is never
    > processed, even though resno=2 with Const(100) is right there in the
    > targetlist.  The Var(3, 2) survives into the planner and would cause
    > problems.
    > 
    > It could be made to work by pre-resolving the generation expressions'
    > base column Vars before adding them to the UPDATE's targetlist.  For
    > each generated column, we'd call ReplaceVarsFromTargetList on the
    > generation expression to resolve its base column Vars, then add the
    > fully resolved expression to the targetlist.  But this seems to add
    > code complexity.  And I'm not sure about the performance difference
    > between these two approaches.  I expect that rule action trees are
    > typically small.
    > 
    > - Richard
    
    My implementation has pre-resolved the generation expressions, that’s why all tests passed. But I agree my change is heavier as I had to add a new static helper function.
    
    If we think rule actions are usually small enough that the extra full-tree pass would not be an issue, then v1 may be preferable for simplicity.
    
    My only comment on v1 is the typo in generated_virtual.sql where “STORED” should be “VIRTUAL”.
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
    
    
    
  12. Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2026-04-17T10:35:17Z

    On Tue, 14 Apr 2026 at 07:29, Chao Li <li.evan.chao@gmail.com> wrote:
    >
    > > On Apr 14, 2026, at 11:27, Richard Guo <guofenglinux@gmail.com> wrote:
    > >
    > > It could be made to work by pre-resolving the generation expressions'
    > > base column Vars before adding them to the UPDATE's targetlist.  For
    > > each generated column, we'd call ReplaceVarsFromTargetList on the
    > > generation expression to resolve its base column Vars, then add the
    > > fully resolved expression to the targetlist.  But this seems to add
    > > code complexity.  And I'm not sure about the performance difference
    > > between these two approaches.  I expect that rule action trees are
    > > typically small.
    > >
    > My implementation has pre-resolved the generation expressions, that’s why all tests passed. But I agree my change is heavier as I had to add a new static helper function.
    >
    > If we think rule actions are usually small enough that the extra full-tree pass would not be an issue, then v1 may be preferable for simplicity.
    >
    > My only comment on v1 is the typo in generated_virtual.sql where “STORED” should be “VIRTUAL”.
    >
    
    I don't quite buy the argument that the rule action is typically
    small. We have no idea how big it might be. Note that, at that point
    in the code, sub_action is the combination of both the original query
    and the rule action.
    
    I do accept though that rules are not widely used, and that it's not
    worth optimising too much, if it means a lot of extra complexity.
    However, IMO, it is slightly simpler and neater to put the expanded
    generated columns in the replacement list used by
    ReplaceVarsFromTargetList() on sub_action.
    
    In the attached v2 patch, I've done that by refactoring
    expand_generated_columns_internal(), renaming it to
    get_generated_columns(), and making it just return the list of
    generated column expressions, rather than doing the rewrite -- I never
    particularly liked the separation of concerns between
    expand_generated_columns_internal() and
    expand_generated_columns_in_expr(), especially after the rest of the
    code expanding virtual generated columns was moved out of the
    rewriter, so that expand_generated_columns_in_expr() became the only
    caller of expand_generated_columns_internal(). Doing this simplifies
    the function, since it's no longer necessary to pass it node, rte, and
    result_relation.
    
    With that change, all rewriteRuleAction() needs to do is get the
    generated columns, rewrite any new.attribute references in them, and
    then use that list plus the original target list as the replacement
    list when rewriting sub_action.
    
    It is a slightly bigger patch overall, but it feels a little neater
    and more logical to me, but I accept that that's a subjective thing.
    
    Note: I included a minor comment update needed for
    build_generation_expression(), and the test fix noted above.
    
    Regards,
    Dean
    
  13. Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)

    Richard Guo <guofenglinux@gmail.com> — 2026-04-20T05:25:43Z

    On Fri, Apr 17, 2026 at 7:35 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
    > I don't quite buy the argument that the rule action is typically
    > small. We have no idea how big it might be. Note that, at that point
    > in the code, sub_action is the combination of both the original query
    > and the rule action.
    >
    > I do accept though that rules are not widely used, and that it's not
    > worth optimising too much, if it means a lot of extra complexity.
    > However, IMO, it is slightly simpler and neater to put the expanded
    > generated columns in the replacement list used by
    > ReplaceVarsFromTargetList() on sub_action.
    
    Fair point.
    
    > In the attached v2 patch, I've done that by refactoring
    > expand_generated_columns_internal(), renaming it to
    > get_generated_columns(), and making it just return the list of
    > generated column expressions, rather than doing the rewrite -- I never
    > particularly liked the separation of concerns between
    > expand_generated_columns_internal() and
    > expand_generated_columns_in_expr(), especially after the rest of the
    > code expanding virtual generated columns was moved out of the
    > rewriter, so that expand_generated_columns_in_expr() became the only
    > caller of expand_generated_columns_internal(). Doing this simplifies
    > the function, since it's no longer necessary to pass it node, rte, and
    > result_relation.
    >
    > With that change, all rewriteRuleAction() needs to do is get the
    > generated columns, rewrite any new.attribute references in them, and
    > then use that list plus the original target list as the replacement
    > list when rewriting sub_action.
    
    Yeah, this is a better approach.  The change looks good to me.
    
    A nitpick: For the comment "The generated column expressions typically
    refer to new.attribute ...", maybe we can remove "typically", as
    generation expressions always refer to columns of the same relation.
    
    - Richard
    
    
    
    
  14. Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)

    Richard Guo <guofenglinux@gmail.com> — 2026-04-20T07:29:04Z

    On Mon, Apr 20, 2026 at 2:25 PM Richard Guo <guofenglinux@gmail.com> wrote:
    > Yeah, this is a better approach.  The change looks good to me.
    >
    > A nitpick: For the comment "The generated column expressions typically
    > refer to new.attribute ...", maybe we can remove "typically", as
    > generation expressions always refer to columns of the same relation.
    
    I noticed a couple of issues after a further look.
    
    1. The ReplaceVarsFromTargetList call on "gen_cols" fails to handle
    hasSubLinks.  This can cause error if targetList contains SubLinks:
    
    update t set a = (select max(a) from t);
    ERROR:  replace_rte_variables inserted a SubLink, but has noplace to record it
    
    2. The same bug fixed in this patch also exists in rule quals:
    
    create table t (a int, b int generated always as (a *2));
    insert into t values (1);
    
    create rule rule_qual as on update to t where new.b > 100
      do instead nothing;
    
    update t set a = 100;
    
    select * from t;
      a  |  b
    -----+-----
     100 | 200
    (1 row)
    
    I think we should apply the same fix to CopyAndAddInvertedQual.
    
    Attached v3 fixes them.
    
    - Richard
    
  15. Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2026-04-20T08:42:31Z

    On Mon, 20 Apr 2026 at 08:29, Richard Guo <guofenglinux@gmail.com> wrote:
    >
    > I noticed a couple of issues after a further look.
    >
    > 1. The ReplaceVarsFromTargetList call on "gen_cols" fails to handle
    > hasSubLinks.  This can cause error if targetList contains SubLinks:
    >
    > update t set a = (select max(a) from t);
    > ERROR:  replace_rte_variables inserted a SubLink, but has noplace to record it
    >
    
    Doh, yes, careless copy-and-pasting. The updates look good.
    
    > 2. The same bug fixed in this patch also exists in rule quals:
    >
    > I think we should apply the same fix to CopyAndAddInvertedQual.
    >
    
    Agreed.
    
    > Attached v3 fixes them.
    >
    
    LGTM.
    
    Regards,
    Dean
    
    
    
    
  16. Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value)

    Richard Guo <guofenglinux@gmail.com> — 2026-04-21T06:05:07Z

    On Mon, Apr 20, 2026 at 5:42 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
    > On Mon, 20 Apr 2026 at 08:29, Richard Guo <guofenglinux@gmail.com> wrote:
    > > Attached v3 fixes them.
    
    > LGTM.
    
    Thanks!  I've pushed and back-patched to v14 (all supported branches).
    
    Virtual generated columns and some helper functions such as
    build_generation_expression were added in v18, so the back-patches in
    pre-v18 branches differ slightly from the v18+ patches.
    
    - Richard