[warning, reviving a thread from 2018] >>>>> "Andrew" == Andrew Dunstan <andrew.duns...@2ndquadrant.com> writes:
> On Wed, Feb 21, 2018 at 7:48 PM, Andres Freund <and...@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)