Re: gen_guc_tables.pl: Validate required GUC fields before code generation - Mailing list pgsql-hackers

Added to CF: https://commitfest.postgresql.org/patch/6226/

> On Nov 12, 2025, at 09:10, Chao Li <li.evan.chao@gmail.com> wrote:
>
>
>
> 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.datguc_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.datguc_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.datguc_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/
>  <v2-0001-gen_guc_tables.pl-Validate-required-GUC-fields-be.patch>

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