Hi Amul,
On Wed, Aug 2, 2023 at 4:06 PM Amul Sul <sula...@gmail.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
