Re: Towards easier AMs: Cleaning up inappropriate use of name "relkind"

On Sat, Jul 11, 2020 at 03:32:55PM -0400, Tom Lane wrote:
> Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com> writes:
>> There are code paths where relkind is sometimes '\0' under normal,
>> non-exceptional conditions. This happens in
>>
>> allpaths.c: set_append_rel_size
>> rewriteHandler.c: view_query_is_auto_updatable
>> lockcmds.c: LockViewRecurse_walker
>> pg_depend.c: getOwnedSequences_internal

There are more code paths than what's mentioned upthread when it comes
to relkinds and \0. For example, I can quickly grep for acl.c that
relies on get_rel_relkind() returning \0 when the relkind cannot be
found. And we do that for get_typtype() as well in the syscache.

>> Doesn't this justify having RELKIND_NULL in the enum?
>
> I'd say no. I think including an intentionally invalid value in such
> an enum is horrid, mainly because it will force a lot of places to cover
> that value when they shouldn't (or else draw "enum value not handled in
> switch" warnings). The confusion factor about whether it maybe *is*
> a valid value is not to be discounted, either.

I agree here that the situation could be improved because we never
store this value in the catalogs. Perhaps there would be a benefit in
switching to an enum in the long run, I am not sure. But if we do so,
RELKIND_NULL should not be around.
--
Michael