Horton

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

jian he <jian.universality@gmail.com>

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

Attachments

On Mon, Mar 3, 2025 at 4:45 PM jian he <jian.universality@gmail.com> wrote:
>
> looking at DefineRelation comments:
>      * We can set the atthasdef flags now in the tuple descriptor; this just
>      * saves StoreAttrDefault from having to do an immediate update of the
>      * pg_attribute rows.
> this seems not right?
> DefineRelation->heap_create_with_catalog->heap_create->RelationBuildLocalRelation->CreateTupleDescCopy
> don't copy atthasdef.
> RelationBuildLocalRelation later didn't touch atthasdef.
> populate_compact_attribute didn't touch atthasdef.
> so StoreAttrDefault has to update that pg_attribute row.
>
CREATE TABLE (COLUMN x DEFAULT y):
for each (Relation rel, AttrNumber attnum), we can enter StoreAttrDefault once.
Also for the above quoted reason,
in DefineRelation, we didn't change pg_attribute.atthasdef before
entering StoreAttrDefault.

ALTER TABLE ALTER COLUMN SET/DROP DEFAULT:
in ATExecColumnDefault will first do RemoveAttrDefault, then re add
the default expression.
In this case, in StoreAttrDefault: attStruct->atthasdef will be false.

overall, i think DefineRelation about StoreAttrDefault comments can be removed.
and StoreAttrDefault, "if (!attStruct->atthasdef)" part logic can also
be changed.

The attached is the minor change I came up with, based on v3-0001 and v3-0002.

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.