Re: Virtual generated columns

From: Richard Guo <guofenglinux(at)gmail(dot)com>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: Alexander Lakhin <exclusion(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>, Zhang Mingli <zmlpostgres(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Virtual generated columns
Date: 2025-05-29 03:06:33
Message-ID: CAMbWs49QZ7nOv-YfsyLpxSPvP1XhDgen5yXn2pWWvqcurT=0LQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, May 16, 2025 at 5:35 PM jian he <jian(dot)universality(at)gmail(dot)com> 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.

Thanks
Richard

Attachment Content-Type Size
v1-0001-Expand-virtual-generated-columns-for-ALTER-COLUMN.patch application/octet-stream 5.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message shveta malik 2025-05-29 03:09:25 Re: Replication slot is not able to sync up
Previous Message Bruce Momjian 2025-05-29 02:49:47 Re: PG 18 release notes draft committed