Re: gen_guc_tables.pl: Validate required GUC fields before code generation

On Mon, Nov 10, 2025 at 8:02 PM Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
wrote:

> Chao Li <li.evan.chao@gmail.com> writes:
>
> > Hi Hacker,
> >
> > While working on the other patch and editing guc_parameters.dat, I
> > mistakenly deleted a “max” value line, then I got the following error
> > during build:
> >
> > ```
> > '/opt/homebrew/bin/perl' ../../../src/backend/utils/misc/
> gen_guc_tables.pl
> > ../../../src/backend/utils/misc/guc_parameters.dat guc_tables.inc.c
> > Use of uninitialized value in printf at ../../../src/backend/utils/misc/
> > gen_guc_tables.pl line 78.
> > make[2]: *** [guc_tables.inc.c] Error 25
> > ```
>
> Yeah, that's not a very helpful error.
>
> > The error message is unclear and is not helpful to identify the real
> issue.
> >
> > I am not an expert of perl, but I tried to make an enhancement that will
> > print an error message like:
>
> The patch looks good overall, but it could be made a bit more concise
> (without sacrificing readability, IMO):
>
> > +sub validate_guc_entry
> > +{
> > +    my ($entry) = @_;
> > +
> > +    my @required_common = qw(name type context group short_desc
> variable boot_val);
> > +
> > +    # Type-specific required fields
> > +    my %required_by_type = (
> > +        int  => [qw(min max)],
> > +        real => [qw(min max)],
> > +        enum => [qw(options)],
> > +        bool => [],   # no extra required fields
> > +        string => [], # no extra required fields
> > +    );
>
> Since we're checking if the key exists, there's no point in having these
> empty arrays for types that don't have any extra fields.
>

Deleted bool and string.


>
> > +    # Check common required fields
> > +    for my $f (@required_common) {
> > +        unless (defined $entry->{$f}) {
> > +            die sprintf("error: guc_parameters.data: '%s' is missing
> required field '%s'\n",
> > +                        $entry->{name} // '<unknown>', $f);
> > +        }
> > +    }
> > +
> > +    # Check type-specific fields
> > +    if (exists $required_by_type{$entry->{type}}) {
> > +        for my $f (@{ $required_by_type{$entry->{type}} }) {
> > +            unless (defined $entry->{$f}) {
> > +                die sprintf("error: guc_parameters.data: '%s' of type
> '%s' is missing required field '%s'\n",
> > +                            $entry->{name}, $entry->{type}, $f);
> > +            }
> > +        }
> > +    }
> > +}
>
> These two loops could be combined, with something like:
>
>     for my $f (@required_common, @{ $required_by_type{$entry->{type}} //
> [] }) {
>
> The `// []` makes it default to an empty array for types that don't have
> any extra fields, so we don't need to list them in the %required_by_type
> hash.  OTOH, we could instead throw an error if there's no entry, to
> catch typos in the type field here, instead of only when we get around
> to compiling the generated file. But that should IMO be done after the
> checking for required fields, so we want the fallback here even if we
> have the empty arrays.
>

Combined the two loops.


>
> There's no need for a `// ''` on the `->{type}` lookup, since it
> would have died while checking the common fields if that were missing.
>
>
Based on my test, a `// ''` on the `->{type}` lookup is still needed,
otherwise if I remove a "type" field, then I got:

```
'/opt/homebrew/bin/perl' ../../../src/backend/utils/misc/gen_guc_tables.pl
../../../src/backend/utils/misc/guc_parameters.dat guc_tables.inc.c
Use of uninitialized value in hash element at
../../../src/backend/utils/misc/gen_guc_tables.pl line 55.
```

In v2, I also added line number to the error message, because I found that
if "name" field is missing or typo on "name", then the error message
without a line number doesn't help either, like:
```
'/opt/homebrew/bin/perl' ../../../src/backend/utils/misc/gen_guc_tables.pl
../../../src/backend/utils/misc/guc_parameters.dat guc_tables.inc.c
error: guc_parameters.data: '<unknown>' of type 'int' is missing required
field 'name'
```

With a line number, we can easily identify the error place:
```
'/opt/homebrew/bin/perl' ../../../src/backend/utils/misc/gen_guc_tables.pl
../../../src/backend/utils/misc/guc_parameters.dat guc_tables.inc.c
error: guc_parameters.data line 1882: '<unknown>' of type 'int' is missing
required field 'name'
```

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