Re: ALTER TABLE ADD COLUMN fast default

On 4/3/21 10:09 PM, Tom Lane wrote:
> Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk> writes:
>> I just got through diagnosing a SEGV crash with someone on IRC, and the
>> cause turned out to be exactly this - a table had (for some reason we
>> could not determine within the available resources) lost its pg_attrdef
>> record for the one column it had with a default (which was a serial
>> column, so none of the fast-default code is actually implicated).

I don't recall why the check was removed. In general the fast default
code doesn't touch adbin.

I'd be curious to know how this state came about. From the description
it sounds like something removed the pg_attrdef entry without adjusting
pg_attribute, which shouldn't happen.

>> Any
>> attempt to alter the table resulted in a crash in equalTupleDesc on this
>> line:
>> if (strcmp(defval1->adbin, defval2->adbin) != 0)
>> due to trying to compare adbin values which were NULL pointers.
> Ouch.
>
>> Does equalTupleDesc need to be more defensive about this, or does the
>> above check need to be reinstated?
> Looking around at the other touches of AttrDefault.adbin in the backend
> (of which there are not that many), some of them are prepared for it to be
> NULL and some are not. I don't immediately have a strong opinion whether
> that should be an allowed state; but if it is not allowed then it's not
> okay for AttrDefaultFetch to leave such a state behind. Alternatively,
> if it is allowed then equalTupleDesc is in the wrong. We should choose
> one definition and make all the relevant code match it.
>
>

There's already a warning if it sets an explicit NULL value, so I'm
inclined to say your suggestion "it's not okay for AttrDefaultFetch to
leave such a state behind" is what we should go with.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com