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).
> /*
> * 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.
--
Álvaro Herrera 39°49'30"S 73°17'W
"Uno puede defenderse de los ataques; contra los elogios se esta indefenso"