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.
