On Fri, Aug 25, 2023 at 5:35 AM Vik Fearing <v...@postgresfriends.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
