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