Re: bug when apply fast default mechanism for adding new column over domain with default value

On Sun, Mar 2, 2025 at 7:54 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> jian he <jian(dot)universality(at)gmail(dot)com> writes:
> > So I duplicated StoreAttrDefault and removed pg_attribute.atthasdef,
> > pg_attrdef related code.
> > and it works fine.
>
> I think the fundamental problem here is that StoreAttrDefault
> shouldn't be involved in this in the first place. It looks like
> somebody did that in the hopes of avoiding two updates of the
> new pg_attribute row (one to set atthasdef and the other to fill
> attmissingval), but it's just bad code structure. We should
> take that code out of StoreAttrDefault, not duplicate it, because
> the assumption that the missingval is identical to what goes into
> pg_attrdef is just wrong.
>

in StoreAttrDefault,
i've split missingval related code into StoreAttrMissingVal.

> Also, since the "if (RELKIND_HAS_STORAGE(relkind))" stanza
> in ATExecAddColumn is already calling expression_planner,
> we should be able to avoid doing that twice on the way to
> discovering whether the expression is a constant.
done.

> I kind of feel that StoreAttrMissingVal belongs in heap.c;
> it's got about nothing to do with pg_attrdef.
heap.c seems fine.