Horton

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

Vaibhav Dalvi <vaibhav.dalvi@enterprisedb.com>

View thread
From: Vaibhav Dalvi <vaibhav.dalvi@enterprisedb.com>
To: Amul Sul <sulamul@gmail.com>
Cc: pgsql-hackers@postgresql.org
Date: 2023-08-24T04:06:12Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. ALTER TABLE command to change generation expression

  2. Refactor: separate function to find all objects depending on a column

  3. Turn AT_PASS_* macros into an enum

Hi Amul,


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