> On Feb 2, 2026, at 08:42, Álvaro Herrera <[email protected]> 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 be easy 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’s much harder to forget defining
the corresponding _CHAR_ macro. With this approach, patch 0002 would only need
to replace RELKIND_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 we also have several other types
following a similar pattern, like RelAggInfo, RelCacheInitLock, RelFileNumber,
etc. Using RelKind would seem a bit more consistent with those.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
