public inbox for [email protected]
help / color / mirror / Atom feedFrom: Zsolt Parragi <[email protected]>
To: Chao Li <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: gen_guc_tables improvements
Date: Mon, 16 Mar 2026 05:57:39 +0000
Message-ID: <CAN4CZFNSTSLQ5=haC80i79LR+=bGMbzgfkEtFvPFXZS2JTgBBg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAN4CZFP=3xUoXb9jpn5OWwicg+rbyrca8-tVmgJsQAa4+OExkw@mail.gmail.com>
<[email protected]>
On Sun, Mar 15, 2026 at 10:27 PM Michael Paquier <[email protected]> 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 AM Chao Li <[email protected]> wrote:
> 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)
view thread (4+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: gen_guc_tables improvements
In-Reply-To: <CAN4CZFNSTSLQ5=haC80i79LR+=bGMbzgfkEtFvPFXZS2JTgBBg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox