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

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.