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

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