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 |
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 |