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

From: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
To: Amul Sul <sulamul(at)gmail(dot)com>
Cc: Maxim Orlov <orlovmg(at)gmail(dot)com>, Vaibhav Dalvi <vaibhav(dot)dalvi(at)enterprisedb(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: ALTER COLUMN ... SET EXPRESSION to alter stored generated column's expression
Date: 2023-09-14 13:53:19
Message-ID: CAExHW5u4XLP6ctvZ94XZwQvVBezW4PqpXrrTwsFqwpFs6buhXA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Amul,
I share others opinion that this feature is useful.

>> On Fri, 25 Aug 2023 at 03:06, Vik Fearing <vik(at)postgresfriends(dot)org> wrote:
>>>
>>>
>>> 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.
>>
>>
>> But I have to agree with Vik Fearing, we can make this patch better, should we?
>> I totally understand your intentions to keep the code flow simple and reuse existing code as much
>> as possible. But in terms of semantics of these commands, they are quite different from each other.
>> And in terms of reading of the code, this makes it even harder to understand what is going on here.
>> So, in my view, consider split these commands.
>
>
> Ok, probably, I would work in that direction. I did the same thing that
> SET/DROP DEFAULT does, despite semantic differences, and also, if I am not
> missing anything, the code complexity should be the same as that.

If we allow SET EXPRESSION to convert a non-generated column to a
generated one, the current way of handling ONLY would yield mismatch
between parent and child. That's not allowed as per the documentation
[1]. In that sense not allowing SET to change the GENERATED status is
better. I think that can be added as a V2 feature, if it overly
complicates the patch Or at least till a point that becomes part of
SQL standard.

I think V1 patch can focus on changing the expression of a column
which is already a generated column.

Regarding code, I think we should place it where it's reasonable -
following precedence is usually good. But I haven't reviewed the code
to comment on it.

[1] https://www.postgresql.org/docs/16/ddl-generated-columns.html

--
Best Wishes,
Ashutosh Bapat

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nathan Bossart 2023-09-14 14:57:57 Re: bug fix and documentation improvement about vacuumdb
Previous Message Euler Taveira 2023-09-14 13:21:59 Re: Is it possible to change wal_level online