Re: BUG #17056: Segmentation fault on altering the type of the foreign table column with a default

On 6/14/21 3:13 PM, Alvaro Herrera wrote:
> On 2021-Jun-12, Andrew Dunstan wrote:
>
>> +    /* Don't do anything unless it's a RELKIND type relation */
>> +    if (tablerel->rd_rel->relkind != RELKIND_RELATION)
>> +    {
>> +        table_close(tablerel, AccessExclusiveLock);
>> +        return;
>> +    }
> "RELKIND type relation" is the wrong phrase ... maybe "it's a plain
> table" is good enough?  (Ditto in RelationBuildTupleDesc).



OK, will change.



>
>>      /*
>>       * Here we go --- change the recorded column type and collation.  (Note
>>       * heapTup is a copy of the syscache entry, so okay to scribble on.) First
>> -     * fix up the missing value if any.
>> +     * fix up the missing value if any. There shouldn't be any missing values
>> +     * for anything except RELKIND_RELATION relations, but if there are, ignore
>> +     * them.
>>       */
>> -    if (attTup->atthasmissing)
>> +    if (rel->rd_rel->relkind == RELKIND_RELATION  && attTup->atthasmissing)
> Would it be sensible to have a macro "AttributeHasMissingVal(rel,
> attTup)", to use instead of reading atthasmissing directly?  The macro
> would check the relkind, and also serve as documentation that said check
> is necessary.



Well AFAIK this is the only place we actually need this combination of
tests, and I'm not a huge fan of defining a macro to use in one spot.


cheers


andrew


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