Re: relkind as an enum - Mailing list pgsql-hackers

From Chao Li
Subject Re: relkind as an enum
Date
Msg-id 9E71DB52-9BA3-4B33-90D2-6E0E8580DD52@gmail.com
Whole thread Raw
In response to relkind as an enum  (Álvaro Herrera <alvherre@kurilemu.de>)
List pgsql-hackers

> On Feb 2, 2026, at 08:42, Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> Hi,
>
> There have been mentions of turning Form_pg_class->relkind into an enum,
> so that we can have compilers provide some more help with
> switch(relkind) blocks.  Here's a quick experiment with that.

Sounds like a correct direction to go overall.

>
> The most annoying part of this is that query-generating code uses
> CppAsString2() to turn the char values into strings.  Making that code
> use the enum values directly is quite messy, so before spending real
> time into making that correct, I just added some ugly #define
> RELKIND_x_STR macros, to substitute the uses of the other construct.
> This is not intended to be final form.  This is 0001+0002, both
> mechanical[1].

I’m not a big fan of the new RELKIND_x_STR macros. Not only because there aren’t many similar macros in the existing
codebase,but more importantly because RELKIND_x_STR is effectively decoupled from the enum values RELKIND_x. It would
beeasy in the future to add a new enum value and forget to introduce the corresponding _STR macro. 

What do you think about an approach like this instead?
```
#define RELKIND_CHAR_RELATION ‘r’
…

typedef enum
{
    RELKIND_RELATION = RELKIND_CHAR_RELATION,
    ...
} Relkind;
```

This keeps the enum values mechanically tied to the underlying character constants. When a new enum value is added,
it’smuch harder to forget defining the corresponding _CHAR_ macro. With this approach, patch 0002 would only need to
replaceRELKIND_x with RELKIND_CHAR_x. 

>
> 0003 is the backend-side change.  This looks generally reasonable,
> though I'm annoyed that I couldn't find a way to coerce the compiler
> into telling me if I had missed some spot.  I didn't change any
> switch(relkind) blocks (except one in pg_overexplain which causes a
> compiler warning for trying to use the non-existent '\0' value), but I
> played with a couple of them by removing the default clauses and indeed
> we now get warnings for missing cases.
>
> Of course, pg_class.relkind itself (the on-disk catalog) continues to be
> a single char with the same values as before.
>
> Does this look more or less a direction we'd like to go in?
>
> [1] git grep --files-with-matches 'CppAsString2(RELKIND' | xargs -n1 perl -pi -e
's/CppAsString2\((RELKIND[^)]*)\)/$1_STR/g'
>

One minor naming question as well: why Relkind instead of RelKind? There’s already a type named PublicationRelKind, and
wealso have several other types following a similar pattern, like RelAggInfo, RelCacheInitLock, RelFileNumber, etc.
UsingRelKind would seem a bit more consistent with those. 

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/







pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: AIX support
Next
From: Xuneng Zhou
Date:
Subject: Re: Wake up backends immediately when sync standbys decrease