On Fri, May 16, 2025 at 5:35 PM jian he <jian.universal...@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
v1-0001-Expand-virtual-generated-columns-for-ALTER-COLUMN.patch
Description: Binary data