Re: ALTER TABLE ADD COLUMN fast default - Mailing list pgsql-hackers

[warning, reviving a thread from 2018]

>>>>> "Andrew" == Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

 > On Wed, Feb 21, 2018 at 7:48 PM, Andres Freund <andres@anarazel.de> wrote:
 >> Hi,

 Andrew> Comments interspersed.

 >>> @@ -4059,10 +4142,6 @@ AttrDefaultFetch(Relation relation)
 >>> 
 >>> systable_endscan(adscan);
 >>> heap_close(adrel, AccessShareLock);
 >>> -
 >>> -     if (found != ndef)
 >>> -             elog(WARNING, "%d attrdef record(s) missing for rel %s",
 >>> -                      ndef - found, RelationGetRelationName(relation));
 >>> }
 >> 
 >> Hm, it's not obvious why this is a good thing?

I didn't find an answer to this in the thread archive, and the above
change apparently did make it into the final patch.

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). 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.

So, questions: why was the check removed in the first place?

(Why was it previously only a warning when it causes a crash further
down the line on any alteration?)

Does equalTupleDesc need to be more defensive about this, or does the
above check need to be reinstated?

(The immediate issue was fixed by "update pg_attribute set
atthasdef=false ..." for the offending attribute and then adding it back
with ALTER TABLE, which seems to have cured the crash.)

-- 
Andrew (irc:RhodiumToad)