Re: relkind as an enum

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: Álvaro Herrera <alvherre(at)kurilemu(dot)de>
Cc: Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: relkind as an enum
Date: 2026-02-02 02:26:28
Message-ID: 9E71DB52-9BA3-4B33-90D2-6E0E8580DD52@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Feb 2, 2026, at 08:42, Álvaro Herrera <alvherre(at)kurilemu(dot)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 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/

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Xuneng Zhou 2026-02-02 02:28:05 Re: Wake up backends immediately when sync standbys decrease
Previous Message Tom Lane 2026-02-02 02:18:08 Re: AIX support