Re: Towards easier AMs: Cleaning up inappropriate use of name "relkind" - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Towards easier AMs: Cleaning up inappropriate use of name "relkind"
Date
Msg-id 20200712115939.GC21680@paquier.xyz
Whole thread Raw
In response to Re: Towards easier AMs: Cleaning up inappropriate use of name "relkind"  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Towards easier AMs: Cleaning up inappropriate use of name "relkind"  (Mark Dilger <mark.dilger@enterprisedb.com>)
List pgsql-hackers
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

Attachment

pgsql-hackers by date:

Previous
From: Rafia Sabih
Date:
Subject: Re: Parallel copy
Next
From: Bharath Rupireddy
Date:
Subject: Re: Parallel copy