MIME-Version: 1.0
References: 
 <CAN4CZFP=3xUoXb9jpn5OWwicg+rbyrca8-tVmgJsQAa4+OExkw@mail.gmail.com>
 <41FFF107-16B6-4B5A-BFDF-88AEF4B776F4@gmail.com>
In-Reply-To: <41FFF107-16B6-4B5A-BFDF-88AEF4B776F4@gmail.com>
From: Zsolt Parragi <zsolt.parragi@percona.com>
Date: Mon, 16 Mar 2026 05:57:39 +0000
Message-ID: 
 <CAN4CZFNSTSLQ5=haC80i79LR+=bGMbzgfkEtFvPFXZS2JTgBBg@mail.gmail.com>
Subject: Re: gen_guc_tables improvements
To: Chao Li <li.evan.chao@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAN4CZFNSTSLQ5%3DhaC80i79LR%2B%3DbGMbzgfkEtFvPFXZS2JTgBBg%40mail.gmail.com>
Precedence: bulk

On Sun, Mar 15, 2026 at 10:27=E2=80=AFPM Michael Paquier <michael@paquier.x=
yz> wrote:

> I'd say no on this one, which would mean going back-and-forth with the
> input.  If these fail due to a compilation failure, it's not that bad
> either, it would be easy to pin-point that the failures are related to
> the values of the GUC, not the overall shape of the data in the input
> file.  Saying that, it would depend on how much complexity this adds
> and what kind of validation we'd get out of it.  If the additions to
> the script are simple like the ones you are proposing here, that would
> be probably OK if these improvements catch a bunch of ground-breaking
> inconsistencies.  If the whole script would need to be refactored,
> probably not.

We can have two kinds of errors:

1. A typo in an enum name - this will result in a compilation failure
most likely, and the compiler will provide nice error messages with
the fix suggestion (e.g. GUC_NOT_IN_SAMPLE -> GUC_NOT_NI_SAMPLE, or
PGC_SUSET -> PGC_SUSTE)
2. a mismatched enum because of a copy paste mistake (group says
PGC_SUSET instead of WAL_ARCHIVING, or boot_val says WAL_ARCHIVING
instead of ARCHIVE_MODE_OFF). The examples I provided there aren't
likely copy-paste errors, that one is when you copy-paste an existing
enum guc and forget to change its boot value.

(2) currently compiles without warnings in both cases, but it is also
usually easier to spot during review.

The patch itself would be well contained: one or two utility functions
(GUC_NOT_IN_SAMPLE and other flags are defines, not enums), and then
calling them at a few places. So it's not a significant refactoring.

But I'm also not a fan of parsing C source code in perl, that's what I
meant by fragile. It seems to work, but I wouldn't guarantee that it
can properly parse all edge cases. Thinking about this more, these
checks would better be implemented as a custom clang-tidy check for
example, which I already started to look into for another issue.


On Mon, Mar 16, 2026 at 3:13=E2=80=AFAM Chao Li <li.evan.chao@gmail.com> wr=
ote:

> I think dot is allowed in a GUC name. Looking at the C code:

Yes, but by convention dot is only used by extensions to mark the
prefix/namespace of the extension (<extension_name>.<guc_name>). None
of the core server GUCs use it. The point of this script isn't to
ensure that the generated C code will compile, but to prevent hidden
errors and ensure we follow existing proper coding conventions.

> BTW, I have similar patch [1] that verify duplicate GUC enum values.

I forgotten about that, thanks for the reminder! I didn't like the
runtime check approach of it, and I think there are more exceptions
than the whitelisted items in the patch, but adding the ability to
mark a guc value list unique could be another good candidate for a
custom clang-tidy check. (assuming people like my clang-tidy idea,
which I didn't even start a thread about yet)