Re: Add sanity check for duplicate enum values in GUC definitions

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>
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-19 00:20:26
Message-ID: 32DE600C-6A62-4F8D-AA07-CF661E789B5D@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Dec 18, 2025, at 15:52, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
>
>
>> On Dec 18, 2025, at 15:43, Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>>
>> On 18.12.25 01:22, Chao Li wrote:
>>>> On Dec 17, 2025, at 22:51, Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>>>> On 15.12.25 10:16, Chao Li wrote:
>>>>> The motivation for this patch comes from my own experience. While working on [1]. I added an enum-typed GUC and made a copy-and-paste mistake, assigning the same numeric value to two different enum entries. This resulted in confusing runtime behavior and cost me about an hour to track down.
>>>>
>>>> Why do you assign explicit values at all?
>>> Did you mean to say “duplicate” instead of “explicit”?
>>
>> No, I meant explicit. I didn't find an example in the thread you linked to, but I suppose you are writing something like
>>
>> enum foo {
>> bar = 1,
>> baz = 2,
>> };
>>
>> But why make those assignments at all. You could just write
>>
>> enum foo {
>> bar,
>> baz,
>> };
>>
>
> Oh, I got your question. That's not C enum, it’s about the GUC config_enum_entry. In the reply to Zsolt, I explained what I experienced.
>
>> 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.
>

By the way, CF entry: https://commitfest.postgresql.org/patch/6316/

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 Peter Smith 2025-12-19 00:28:31 Re: Proposal: Conflict log history table for Logical Replication
Previous Message Peter Smith 2025-12-19 00:04:47 Re: Proposal: Conflict log history table for Logical Replication