Horton

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

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: jian he <jian.universality@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2025-03-02T22:08:39Z
Lists: pgsql-hackers

Attachments

jian he <jian.universality@gmail.com> writes:
> On Sun, Mar 2, 2025 at 7:54 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I think the fundamental problem here is that StoreAttrDefault
>> shouldn't be involved in this in the first place.

> i've split missingval related code into StoreAttrMissingVal.

Hm, this does nothing to improve the modularity of the affected
code; if anything it's worse than before.  (Fools around for
awhile...)  I had something more like the attached in mind.
The idea here is to centralize the control of whether we are
storing a missingval or doing a table rewrite in ATExecAddColumn.
StoreAttrMissingVal has nothing to do with pg_attrdef nor does
StoreAttrDefault have anything to do with attmissingval.

I looked briefly at determining the presence of a default
earlier so we could avoid the extra update of pg_attribute
when both those changes need to happen, but I concluded that
it'd take more refactoring than it's worth.  The problem is
the logic way down inside AddRelationNewConstraints that
checks for the eventually-cooked default expression just
being a null constant.

			regards, tom lane

Commits

  1. Need to do CommandCounterIncrement after StoreAttrMissingVal.

  2. Simplify some logic around setting pg_attribute.atthasdef.

  3. Remove now-dead code in StoreAttrDefault().

  4. Fix broken handling of domains in atthasmissing logic.