| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
| Cc: | Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Add sanity check for duplicate enum values in GUC definitions |
| Date: | 2025-12-18 00:54:54 |
| Message-ID: | BA390B42-CC22-48BF-845D-DDCED87580DC@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Dec 18, 2025, at 05:32, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> wrote:
>
> Hello
>
>> . While working on [1]. I added an enum-typed GUC
>
> I wanted to check the original issue, but the linked patch adds a
> boolean GUC (logical_replication_fallback_to_full_identity), I did not
> see enum mentioned anywhere in the diff, did you link the correct
> thread?
>
>> Ideally, such a check would run at compile time, but that would require parsing guc_tables.c with Perl
>
> Maybe you could compile a utility executable as part of the build, and
> execute it as part of the test suite?
Hi Zsolt,
Thanks for asking. The link was correct. While working on the patch, I experimented with multiple solutions, one was adding a new GUC “default_replica_identity”.
For that, I defined a enum in guc_table.c, with items like:
```
“Default”, DEFAULT, false,
“Full”, FULL, false,
“None”, FULL, false, <== copy-paste mistake here
NULL, NULL, tue
```
I mistakenly copy FULL to the “None” line. While testing, I did “alter database xxx set default_replica_identity = full/none”, and found that resulted the same. Mixing the fact that a GUC change doesn't take effective immediately, sometimes needing restart/reconnect, etc., I spent time tracking down the error, and finally identified the copy-paste mistake. The experience triggered the idea of adding a sanity check. With this patch, such mistake will cause postmaster fail to start, so that a developer will notice the problem in the first place. That’s why I mentioned this could be a developer-facing feature, maybe put all code inside #ifdef USE_ASSERT_CHECKING, so that it won’t impact release version at all.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Langote | 2025-12-18 01:10:18 | Re: Segmentation fault on proc exit after dshash_find_or_insert |
| Previous Message | Chao Li | 2025-12-18 00:36:23 | Re: Add sanity check for duplicate enum values in GUC definitions |