Re: gen_guc_tables improvements - Mailing list pgsql-hackers

> On Mar 15, 2026, at 20:18, Zsolt Parragi <zsolt.parragi@percona.com> wrote:
>
> Hello
>
> While reviewing a patch, I noticed a typo in guc_params.dat. The code
> compiled and seemingly worked, and I was very surprised that the
> generator script didn't catch the mistake.
>
> I looked into it, and I found several missing checks in
> gen_guc_tables. I attached fixes for 4 that I think would definitely
> improve the script (for now as separate patches, so it is easy to
> select only some of them):
>
> * 0001 fixes the issue that started this, it validates the allowed
> field names, preventing typos in their names
> * 0002 goes a step further and validates that fields specific to some
> types can only appear for those types
> * 0003 just improves the error reported by duplicate names, previously
> this was confusing (it referred to incorrect ordering)
> * 0004 adds basic checks about allowed characters in GUC names
>
> I was also thinking about adding validations for the enum/define
> values (config group, flags, guc context), but that requires a
> somewhat fragile extraction code, and I decided to leave that out for
> now.
>
> What do you think about these changes?
>
<0001-gen_guc_tables-reject-unrecognized-field-names-in-gu.patch><0002-gen_guc_tables-reject-type-inappropriate-fields-in-g.patch><0003-gen_guc_tables-report-duplicate-entry-names-distinct.patch><0004-gen_guc_tables-validate-GUC-name-format.patch>

I reviewed the patch and played a little bit. Overall looks good to me.

My only comment is on 0004:
```
+    unless ($entry->{name} =~ /^[a-zA-Z][a-zA-Z0-9_]*$/)
+    {
+        die sprintf(
+            qq{%s:%d: error: entry name "%s" is not a valid GUC name (must start with a letter, contain only letters,
digits,and underscores)\n}, 
+            $input_fname, $entry->{line_number}, $entry->{name});
+    }
+
```

I think dot is allowed in a GUC name. Looking at the C code:
```
/*
* Decide whether a proposed custom variable name is allowed.
*
* It must be two or more identifiers separated by dots, where the rules
* for what is an identifier agree with scan.l. (If you change this rule,
* adjust the errdetail in assignable_custom_variable_name().)
*/
static bool
valid_custom_variable_name(const char *name)
```

A GUC name can be dot separated multiple identifiers, the rule this patch used applies to identifiers.

Thought current guc_parameters.dat doesn’t have any GUC whose name contains a dot, but dot should be allowed. For
example,I tried on master branch with adding “.test” to an existing GUC name: 
```
{ name => 'debug_print_raw_parse.test', type => 'bool', context => 'PGC_USERSET', group => 'LOGGING_WHAT',
  short_desc => 'Logs each query\'s raw parse tree.',
  variable => 'Debug_print_raw_parse',
  boot_val => 'false',
},
```

And it works:
```
evantest=# set debug_print_raw_parse.test = 1;
SET
```

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

[1] https://www.postgresql.org/message-id/CAEoWx2nAKPx1N1VzKVHjtwqP%2Bs%3DyMR%2BFdmrh44uVNODg--T03w%40mail.gmail.com


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