Re: Virtual generated columns

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