From: | Amul Sul <sulamul(at)gmail(dot)com> |
---|---|
To: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(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: | 2024-01-08 03:38:31 |
Message-ID: | CAAJ_b95GxLjJOU7Vo9ULWPxuuYRJaR66xE8UYgD_RExOYfAV3A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Jan 5, 2024 at 12:28 AM Peter Eisentraut <peter(at)eisentraut(dot)org>
wrote:
> On 25.12.23 13:10, Amul Sul wrote:
> >
> I have committed this patch set.
>
> I couple of notes:
>
> You had included the moving of the AT_PASS_ADD_COL enum in the first
> patch. This is not a good style. Refactoring patches should not
> include semantic changes. I have moved that change the final patch that
> introduced the new feature.
>
> I did not commit the 0002 patch that renamed some functions. I think
> names like AlterColumn are too general, which makes this renaming
> possibly counterproductive. I don't know a better name, maybe
> AlterTypeOrSimilar, but that's obviously silly. I think leaving it at
> AlterType isn't too bad, since most of the code is indeed for ALTER TYPE
> support. We can reconsider this if we have a better idea.
>
> In RememberAllDependentForRebuilding(), I dropped some of the additional
> errors that you introduced for the AT_SetExpression cases. These didn't
> seem useful. For example, it is not possible for a generated column to
> depend on another generated column, so there is no point checking for
> it. Also, there were no test cases that covered any of these
> situations. If we do want any of these, we should have tests and
> documentation for them.
>
> For the tests that examine the EXPLAIN plans, I had to add an ANALYZE
> after the SET EXPRESSION. Otherwise, I got unstable test results,
> presumably because in some cases the analyze happened in the background.
>
>
Understood.
Thank you for your guidance and the commit.
Regards,
Amul
From | Date | Subject | |
---|---|---|---|
Next Message | vignesh C | 2024-01-08 04:55:34 | Re: Forbid the use of invalidated physical slots in streaming replication. |
Previous Message | Andrei Lepikhov | 2024-01-08 03:12:25 | Re: Multidimensional Histograms |