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

On Fri, Aug 25, 2023 at 5:35 AM Vik Fearing <vik(at)postgresfriends(dot)org> wrote:

> On 8/2/23 12:35, Amul Sul 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
>
> I love this idea. It is something that the standard SQL language is
> lacking and I am submitting a paper to correct that based on this. I
> will know in October what the committee thinks of it. Thanks!
>
>
Great, thank you so much.

> > Note that this form of ALTER is meant to work for the column which is
> > already generated.
>
> Why? SQL does not have a way to convert a non-generated column into a
> generated column, and this seems like as good a way as any.
>

Well, I had to have the same thought but Peter Eisentraut thinks that we
should
have that in a separate patch & I am fine with that.

> To keep the code flow simple, I have renamed the existing function that
> was
> > in use for DROP EXPRESSION so that it can be used for SET EXPRESSION as
> well,
> > which is a similar design as SET/DROP DEFAULT. I kept this renaming code
> > changes in a separate patch to minimize the diff in the main patch.
>
> I don't like this part of the patch at all. Not only is the
> documentation only half baked, but the entire concept of the two
> commands is different. Especially since I believe the command should
> also create a generated column from a non-generated one.
>

I am not sure I understood this, why would that break the documentation
even if
we allow non-generated columns to be generated. This makes the code flow
simple
and doesn't have any issue for the future extension to allow non-generated
columns too.

>
> Is is possible to compare the old and new expressions and no-op if they
> are the same?
>
>
> psql (17devel)
> Type "help" for help.
>
> postgres=# create table t (c integer generated always as (null) stored);
> CREATE TABLE
> postgres=# select relfilenode from pg_class where oid = 't'::regclass;
> relfilenode
> -------------
> 16384
> (1 row)
>
> postgres=# alter table t alter column c set expression (null);
> ALTER TABLE
> postgres=# select relfilenode from pg_class where oid = 't'::regclass;
> relfilenode
> -------------
> 16393
> (1 row)
>
>
> I am not saying we should make every useless case avoid rewriting the
> table, but if there are simple wins, we should take them. (I don't know
> how feasible this is.)
>

I think that is feasible, but I am not sure if we want to do that & add an
extra
code for the case, which is not really breaking anything except making the
system do extra work for the user's thoughtless action.

>
> I think repeating the STORED keyword should be required here to
> future-proof virtual generated columns.
>

Agree, added in the v2 version posted a few minutes ago.

Regards,
Amul