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
