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: