Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.400.21\))
Subject: Re: Bug: Rule actions see wrong values for generated columns (NEW.gen
 reads OLD value)
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: 
 <CAMbWs49DcHqOtd_BzuFcr9Va+tD-2fNs8bGsbH_Zy2bXUTNM1g@mail.gmail.com>
Date: Tue, 14 Apr 2026 14:28:24 +0800
Cc: Dean Rasheed <dean.a.rasheed@gmail.com>,
 SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 Peter Eisentraut <peter@eisentraut.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <8D1CD3EB-BF72-4C73-AF24-D88581AC01BE@gmail.com>
References: 
 <CAHg+QDexGTmCZzx=73gXkY2ZADS6LRhpnU+-8Y_QmrdTS6yUhA@mail.gmail.com>
 <CAMbWs4-SJ6B26ciFu_K_2M1ObDA=GirV87N1BKeAtSRmQGATUA@mail.gmail.com>
 <22B4A33A-99F3-46F5-BE0C-426A9E1D9ABA@gmail.com>
 <CAMbWs48=QrLy8gZprTfCmEtzoaUku54JXvAcnoRSo+OC8vUpwA@mail.gmail.com>
 <CAEZATCWYqyJJJcMNEj-LEvCSwog2H_HS61hTROZLKYuaiy5YTg@mail.gmail.com>
 <CAMbWs49DcHqOtd_BzuFcr9Va+tD-2fNs8bGsbH_Zy2bXUTNM1g@mail.gmail.com>
To: Richard Guo <guofenglinux@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/8D1CD3EB-BF72-4C73-AF24-D88581AC01BE%40gmail.com>
Precedence: bulk


> On Apr 14, 2026, at 11:27, Richard Guo <guofenglinux@gmail.com> wrote:
>=20
> On Mon, Apr 13, 2026 at 8:04=E2=80=AFPM 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.
>=20
>> 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().
>=20
> 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.
>=20
> Take Satya's repro as an example.  If we add the generation expression
> for gen to the UPDATE's targetlist, the list would be:
>=20
>    TargetEntry 1: resno=3D2, expr=3DConst(100)       -- a =3D 100
>    TargetEntry 2: resno=3D3, expr=3DVar(3, 2) * 2    -- gen =3D NEW.a =
* 2
>=20
> When ReplaceVarsFromTargetList processes Var(3, 3) (NEW.gen) in the
> rule action, it finds resno=3D3 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=3D2 with Const(100) is right there in the
> targetlist.  The Var(3, 2) survives into the planner and would cause
> problems.
>=20
> 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.
>=20
> - Richard

My implementation has pre-resolved the generation expressions, that=E2=80=99=
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 =
=E2=80=9CSTORED=E2=80=9D should be =E2=80=9CVIRTUAL=E2=80=9D.

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