Re: ALTER COLUMN ... SET EXPRESSION to alter stored generated column's expression

Hi Amul,

On Wed, Aug 2, 2023 at 4:06 PM Amul Sul <sulamul(at)gmail(dot)com> wrote:

> Hi,
>
> Currently, we have an option to drop the expression of stored generated
> columns
> as:
>
> ALTER [ COLUMN ] column_name DROP EXPRESSION [ IF EXISTS ]
>
> But don't have support to update that expression. The attached patch
> provides
> that as:
>
> ALTER [ COLUMN ] column_name SET EXPRESSION expression
>
> +1 to the idea.

Here are few review comments:.
*0001 patch*
1. Alignment changed for below comment:

> AT_ColumnExpression, /* alter column drop expression */

*00002 patch*
1. EXPRESSION should be added after DEFAULT per alphabetic order?

> + COMPLETE_WITH("(", "COMPRESSION", "EXPRESSION", "DEFAULT", "GENERATED",
> "NOT NULL", "STATISTICS", "STORAGE",
>

2. The variable *isdrop* can be aligned better:

> + bool isdrop = (cmd->def == NULL);
>

3. The AlteredTableInfo structure has member Relation, So need to pass
parameter Relation separately?

> static ObjectAddress ATExecColumnExpression(AlteredTableInfo *tab,
> Relation rel,
> const char *colName, Node *newDefault,
> bool missing_ok, LOCKMODE lockmode);
>

4. Exceeded 80 char limit:

> /*
> * Mark the column as no longer generated. (The atthasdef flag needs to
>

5. Update the comment. Use 'set' along with 'drop':

> AT_ColumnExpression, /* alter column drop expression */

Thanks,
Vaibhav Dalvi