Horton

Thread

  1. Add sanity check for duplicate enum values in GUC definitions

    Chao Li <li.evan.chao@gmail.com> — 2025-12-15T09:16:56Z

    Hi Hackers,

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.

I think adding a sanity check for enum-typed GUCs would help catch this
class of mistake early. Ideally, such a check would run at compile time,
but that would require parsing guc_tables.c with Perl. I’m not very
familiar with Perl, so in this patch I instead added the check when
building the GUC hash table.

There are a few existing enum GUCs that intentionally reuse numeric values
(for example, to provide aliases), so I introduced a small whitelist for
those cases. If a new GUC needs duplicate enum values in the future, its
name can simply be added to that list.

This check is intended as a developer-facing sanity guard to catch mistakes
in GUC enum tables. Given that intent, once a duplicate value is detected
the backend aborts early. From that perspective, it might also make sense
to place this check under #ifdef USE_ASSERT_CHECKING. I’m interested in
hearing thoughts on that approach.

[1]
https://postgr.es/m/CAEoWx2mMorbMwjKbT4YCsjDyL3r9Mp+z0bbK57VZ+OkJTgJQVQ@mail.gmail.com

--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

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

    Peter Eisentraut <peter@eisentraut.org> — 2025-12-17T14:51:58Z

    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?

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

    Andres Freund <andres@anarazel.de> — 2025-12-17T15:19:54Z

    Hi,

On 2025-12-15 17:16:56 +0800, 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.

I think this is something we explicitly do *not* want. It can make plenty
sense to have enums with the same value assigned multiple times. And ending up
with a list of exceptions doesn't strike me as a good use of time.

I just dont' believe this is a frequent enough issue to be worth in-core
infrastructure, particularly in light of it sometimes being actually
intentionally used.

Greetings,

Anres


> I think adding a sanity check for enum-typed GUCs would help catch this
> class of mistake early. Ideally, such a check would run at compile time,
> but that would require parsing guc_tables.c with Perl. I’m not very
> familiar with Perl, so in this patch I instead added the check when
> building the GUC hash table.
> 
> There are a few existing enum GUCs that intentionally reuse numeric values
> (for example, to provide aliases), so I introduced a small whitelist for
> those cases. If a new GUC needs duplicate enum values in the future, its
> name can simply be added to that list.
> 
> This check is intended as a developer-facing sanity guard to catch mistakes
> in GUC enum tables. Given that intent, once a duplicate value is detected
> the backend aborts early. From that perspective, it might also make sense
> to place this check under #ifdef USE_ASSERT_CHECKING. I’m interested in
> hearing thoughts on that approach.
> 
> [1]
> https://postgr.es/m/CAEoWx2mMorbMwjKbT4YCsjDyL3r9Mp+z0bbK57VZ+OkJTgJQVQ@mail.gmail.com
> 
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/

> From 3c2c25d19da3b39f57c7d04b17077e015aa82758 Mon Sep 17 00:00:00 2001
> From: "Chao Li (Evan)" <lic@highgo.com>
> Date: Mon, 15 Dec 2025 17:00:59 +0800
> Subject: [PATCH v1] Add sanity check for duplicate enum values in GUC
>  definitions
> 
> Add a backend startup check that scans enum-type GUCs and reports cases
> where multiple enum labels map to the same numeric value.
> 
> Historically, duplicate enum values have occasionally been introduced
> unintentionally, which can make enum lookup ambiguous and harder to
> reason about.  This change detects such cases early, after the GUC hash
> table is built, and errors out to prevent silently accepting ambiguous
> definitions.
> 
> A small number of existing enum GUCs intentionally reuse values (for
> example, to provide backward-compatible aliases).  These are explicitly
> excluded from the check via a whitelist.
> 
> This check is intended as a developer-facing sanity guard to catch
> mistakes in GUC enum tables.
> 
> Author: Chao Li <lic@highgo.com>
> ---
>  src/backend/utils/misc/guc.c        | 63 +++++++++++++++++++++++++++++
>  src/backend/utils/misc/guc_tables.c | 10 +++++
>  src/include/utils/guc_tables.h      |  2 +
>  3 files changed, 75 insertions(+)
> 
> diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
> index 935c235e1b3..58d1eada183 100644
> --- a/src/backend/utils/misc/guc.c
> +++ b/src/backend/utils/misc/guc.c
> @@ -238,6 +238,7 @@ static int	guc_name_match(const void *key1, const void *key2, Size keysize);
>  static void InitializeGUCOptionsFromEnvironment(void);
>  static void InitializeOneGUCOption(struct config_generic *gconf);
>  static void RemoveGUCFromLists(struct config_generic *gconf);
> +static void check_enum_duplicates(void);
>  static void set_guc_source(struct config_generic *gconf, GucSource newsource);
>  static void pg_timezone_abbrev_initialize(void);
>  static void push_old_value(struct config_generic *gconf, GucAction action);
> @@ -860,6 +861,63 @@ get_guc_variables(int *num_vars)
>  	return result;
>  }
>  
> +/*
> + * If an enum-typed GUC has duplicate values for different names, raise an error.
> + * There are a few exceptions, listed in enum_dup_whitelist.
> + */
> +static void
> +check_enum_duplicates(void)
> +{
> +	HASH_SEQ_STATUS status;
> +	GUCHashEntry *hentry;
> +
> +	Assert(guc_hashtab != NULL);
> +
> +	hash_seq_init(&status, guc_hashtab);
> +	while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
> +	{
> +		const struct config_generic *gconf = hentry->gucvar;
> +
> +		if (gconf->vartype == PGC_ENUM)
> +		{
> +			bool		allow_dups = false;
> +
> +			for (const char *const *p = enum_dup_whitelist; *p; p++)
> +			{
> +				if (strcmp(gconf->name, *p) == 0)
> +				{
> +					allow_dups = true;
> +					break;
> +				}
> +			}
> +			if (allow_dups)
> +				continue;
> +
> +			for (const struct config_enum_entry *opt1 = gconf->_enum.options; opt1->name; opt1++)
> +			{
> +				if (opt1->hidden)
> +					continue;
> +
> +				for (const struct config_enum_entry *opt2 = opt1 + 1; opt2->name; opt2++)
> +				{
> +					if (opt2->hidden)
> +						continue;
> +
> +					if (opt1->val == opt2->val)
> +					{
> +						elog(ERROR,
> +							 "GUC enum \"%s\" has duplicate value %d for \"%s\" and \"%s\"",
> +							 gconf->name,
> +							 opt1->val,
> +							 opt1->name,
> +							 opt2->name);
> +					}
> +				}
> +			}
> +		}
> +	}
> +}
> +
>  
>  /*
>   * Build the GUC hash table.  This is split out so that help_config.c can
> @@ -1420,6 +1478,11 @@ InitializeGUCOptions(void)
>  	 */
>  	build_guc_variables();
>  
> +	/*
> +	 * Check for enum GUCs with duplicate values.
> +	 */
> +	check_enum_duplicates();
> +
>  	/*
>  	 * Load all variables with their compiled-in defaults, and initialize
>  	 * status fields as needed.
> diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
> index f87b558c2c6..0eb8dfdf9ff 100644
> --- a/src/backend/utils/misc/guc_tables.c
> +++ b/src/backend/utils/misc/guc_tables.c
> @@ -764,6 +764,16 @@ const char *const config_type_names[] =
>  	[PGC_ENUM] = "enum",
>  };
>  
> +/*
> + * Enum GUCs where duplicate numeric values are expected, so they will be
> + * skipped in enum sanity checks. Hidden enum values are not checked, as they
> + * are not exposed to users.
> + */
> +const char *enum_dup_whitelist[] = {
> +	"wal_compression",
> +	NULL
> +};
> +
>  StaticAssertDecl(lengthof(config_type_names) == (PGC_ENUM + 1),
>  				 "array length mismatch");
>  
> diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.h
> index 04cc60eb526..04f5330fee4 100644
> --- a/src/include/utils/guc_tables.h
> +++ b/src/include/utils/guc_tables.h
> @@ -327,6 +327,8 @@ extern struct config_generic **get_guc_variables(int *num_vars);
>  
>  extern void build_guc_variables(void);
>  
> +extern const char *enum_dup_whitelist[];
> +
>  /* search in enum options */
>  extern const char *config_enum_lookup_by_value(const struct config_generic *record, int val);
>  extern bool config_enum_lookup_by_name(const struct config_enum *record,
> -- 
> 2.39.5 (Apple Git-154)
> 

Greetings,

Andres Freund

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

    Zsolt Parragi <zsolt.parragi@percona.com> — 2025-12-17T21:32:56Z

    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?

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

    Chao Li <li.evan.chao@gmail.com> — 2025-12-18T00:22:13Z

    
> On Dec 17, 2025, at 22:51, Peter Eisentraut <peter@eisentraut.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?

Hi Peter,

Did you mean to say “duplicate” instead of “explicit”?

Duplicate values assigning to different enum items was a copy-paste mistake I made during development, which wasted my time on debugging the issue. So I wanted to add this sanity check to quickly report such mistake in the first place.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

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

    Chao Li <li.evan.chao@gmail.com> — 2025-12-18T00:36:23Z

    
> On Dec 17, 2025, at 23:19, Andres Freund <andres@anarazel.de> wrote:
> 
> Hi,
> 
> On 2025-12-15 17:16:56 +0800, 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.
> 
> I think this is something we explicitly do *not* want. It can make plenty
> sense to have enums with the same value assigned multiple times. And ending up
> with a list of exceptions doesn't strike me as a good use of time.

There is actually only one enum (“wal_compression”) having duplicate values if excluding hidden items. In my patch, hidden items have been ignored, so the white-list has only one entry.

> 
> I just dont' believe this is a frequent enough issue to be worth in-core
> infrastructure, particularly in light of it sometimes being actually
> intentionally used.
> 

I think copy-paste mistake is easy to make. I have done an enhancement for GUC, see [1], that also helps debugging against such errors. In the discussion, you can see that Alvaro echoed that he made the same mistake.

[1] https://www.postgresql.org/message-id/CAEoWx2%3DoP4LgHi771_OKhPPUS7B-CTqCs%3D%3DuQcNXWrwBoAm5Vg%40mail.gmail.com

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

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

    Chao Li <li.evan.chao@gmail.com> — 2025-12-18T00:54:54Z

    
> On Dec 18, 2025, at 05:32, Zsolt Parragi <zsolt.parragi@percona.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/

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

    Peter Eisentraut <peter@eisentraut.org> — 2025-12-18T07:43:34Z

    On 18.12.25 01:22, Chao Li wrote:
>> On Dec 17, 2025, at 22:51, Peter Eisentraut <peter@eisentraut.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,
};

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

    Chao Li <li.evan.chao@gmail.com> — 2025-12-18T07:52:17Z

    
> On Dec 18, 2025, at 15:43, Peter Eisentraut <peter@eisentraut.org> wrote:
> 
> On 18.12.25 01:22, Chao Li wrote:
>>> On Dec 17, 2025, at 22:51, Peter Eisentraut <peter@eisentraut.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.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

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

    Chao Li <li.evan.chao@gmail.com> — 2025-12-19T00:20:26Z

    
> On Dec 18, 2025, at 15:52, Chao Li <li.evan.chao@gmail.com> wrote:
> 
> 
> 
>> On Dec 18, 2025, at 15:43, Peter Eisentraut <peter@eisentraut.org> wrote:
>> 
>> On 18.12.25 01:22, Chao Li wrote:
>>>> On Dec 17, 2025, at 22:51, Peter Eisentraut <peter@eisentraut.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/