On 15.11.23 13:26, Amul Sul wrote:
> Question: Why are you using AT_PASS_ADD_OTHERCONSTR? I don't know if
> it's right or wrong, but if you have a specific reason, it would be
> good
> to know.
>
> I referred to ALTER COLUMN DEFAULT and used that.
Hmm, I'm not sure if that is a good comparison. For ALTER TABLE, SET
DEFAULT is just a catalog manipulation, it doesn't change any data, so
it's pretty easy. SET EXPRESSION changes data, which other phases might
want to inspect? For example, if you do SET EXPRESSION and add a
constraint in the same ALTER TABLE statement, do those run in the
correct order?
> I think ATExecSetExpression() needs to lock pg_attribute? Did you lose
> that during the refactoring?
>
> I have removed that intentionally since we were not updating anything in
> pg_attribute like ALTER DROP EXPRESSION.
ok
> Tiny comment: The error message in ATExecSetExpression() does not need
> to mention "stored", since it would be also applicable to virtual
> generated columns in the future.
>
> I had to have the same thought, but later decided when we do that
> virtual column thing, we could simply change that. I am fine to do that
> change
> now as well, let me know your thought.
Not a big deal, but I would change it now.
Another small thing I found: In ATExecColumnDefault(), there is an
errhint() that suggests DROP EXPRESSION instead of DROP DEFAULT. You
could now add another hint that suggests SET EXPRESSION instead of SET
DEFAULT.