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.dilger@enterprisedb.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