public inbox for [email protected]  
help / color / mirror / Atom feed
From: jian he <[email protected]>
To: Richard Guo <[email protected]>
Cc: Alexander Lakhin <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Dean Rasheed <[email protected]>
Cc: Zhang Mingli <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Virtual generated columns
Date: Mon, 2 Jun 2025 13:30:58 +0800
Message-ID: <CACJufxEVXKDWyxf-kAtodvOXrTA68O8kC5op0v4Rcpf8ORoPeQ@mail.gmail.com> (raw)
In-Reply-To: <CAMbWs49QZ7nOv-YfsyLpxSPvP1XhDgen5yXn2pWWvqcurT=0LQ@mail.gmail.com>
References: <[email protected]>
	<CAMbWs4-h49s0ehfSq4CnztRLSwOF79ZQUkSPzGCQLNroAG7joA@mail.gmail.com>
	<CACJufxHVsCoxhqeDV973Hny3EtgCGztDJj=KKvErexnH=2k6yQ@mail.gmail.com>
	<12d5cce9-917f-4a99-86d3-715bb3b363bb@Spark>
	<CAMbWs4_xiGiB5Oufdn62cJ=MoiS9cJWLzq=jR_7xnTO8x_oPqg@mail.gmail.com>
	<CACJufxEWnz__Bf-KhVaiisHZd-5JcRRY_xttCKLs_m2xaUJmQA@mail.gmail.com>
	<[email protected]>
	<CAEZATCUpzF0XZGVpuJ-O4WLnDWBmddSc+yMWvg6zsz_ekegfPg@mail.gmail.com>
	<CAMbWs4-JmGEBDucPw8iJ3H8hg-7TkbOqE0WTVzhJ5iyzQyP=-A@mail.gmail.com>
	<CAMbWs4_LRNGAxbQbLOrj+VMy80dUR5v2JQ9fVcre10C4KVYUDg@mail.gmail.com>
	<CAEZATCWhr=FM4X5kCPvVs-g2XEk+ceLsNtBK_zZMkqFn9vUjsw@mail.gmail.com>
	<CAEZATCWsKqCtZ=ud26-gGV3zHt-hjS4OKG43GCBhSaYUWyfKiw@mail.gmail.com>
	<CAMbWs4_Rg-F=jQ7-WfBztnVZG2i5kAUwoQv0O5GbGPzxpjDeTQ@mail.gmail.com>
	<CAEZATCV+msUomqYUc-M70epBn7WppLqiw1z=4u4yf6w4vUECiQ@mail.gmail.com>
	<CAMbWs4_DVtFfonq7eL6Ocor4apV9dBb6wOhJ5iUZPEddKc-_vg@mail.gmail.com>
	<[email protected]>
	<CAMbWs4-ow3ZCj=+07GBr6-umNQ6Bg-HkQAofyXDxZET1aJa63g@mail.gmail.com>
	<CACJufxH2v30vZuP8YNJOqrfoRigc8XU8Bu5rVz67z=OYCyNi8g@mail.gmail.com>
	<CAMbWs49QZ7nOv-YfsyLpxSPvP1XhDgen5yXn2pWWvqcurT=0LQ@mail.gmail.com>

On Thu, May 29, 2025 at 11:06 AM Richard Guo <[email protected]> wrote:
>
> On Fri, May 16, 2025 at 5:35 PM jian he <[email protected]> wrote:
> > we have used the USING expression in ATPrepAlterColumnType,
> > ATColumnChangeRequiresRewrite.
> > expanding it on ATPrepAlterColumnType seems to make more sense?
> >
> > @@ -14467,7 +14467,7 @@ ATPrepAlterColumnType(List **wqueue,
> >                  */
> >                 newval = (NewColumnValue *) palloc0(sizeof(NewColumnValue));
> >                 newval->attnum = attnum;
> > -               newval->expr = (Expr *) transform;
> > +               newval->expr = (Expr *)
> > expand_generated_columns_in_expr(transform, rel, 1);
> >                 newval->is_generated = false;
>
> Yeah, ATPrepAlterColumnType does seem like a better place.  But we
> need to ensure that ATColumnChangeRequiresRewrite sees the expanded
> version of the expression — your proposed change fails to do that.
>
> Additionally, I think we also need to ensure that the virtual
> generated columns are expanded before the expression is fed through
> expression_planner, to ensure it can be successfully transformed into
> an executable form.
>
> Hence, the attached patch.

looks good to me.





view thread (29+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Virtual generated columns
  In-Reply-To: <CACJufxEVXKDWyxf-kAtodvOXrTA68O8kC5op0v4Rcpf8ORoPeQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox