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

Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> writes:

> On Wed, Nov 19, 2025 at 4:33 PM Peter Eisentraut <peter(at)eisentraut(dot)org>
> wrote:
>
>> I find the data structures that you have constructed here barely
>> understandable:
>>
>> my %required_by_type = (
>> int => [qw(min max)],
>> real => [qw(min max)],
>> enum => [qw(options)],
>> );
>>
>> for my $f (@required_common, @{ $required_by_type{$entry->{type} //
>> ''} // [] }) {
>>
>> [qw(min max)] is an array inside an array reference? I think? Do we
>> need two levels of nesting?

No there is no nesting. qw() is a quote operator ("quote words") that
returns a list of the words inside, and [] creates an array of the
elements of the list and returns a reference to it, So [qw(min max)] is
equivalent to ['min', 'max'].

>> I think this // notation is unnecessarily confusing, and why do we need
>> two of them. I thought your first patch
>>
>> + bool => [], # no extra required fields
>> + string => [], # no extra required fields
>>
>> was clearer. And that way, we also check that the field type is one of
>> the ones we support.
>
> Yeah, the two levels of nesting is not necessary. It was to address the
> review comments of v1, I removed bool and string from required_by_type and
> combined the two loops into one. Because bool and string don't exist in
> required_by_type, so that $required_by_type{$entry->{type} needs a
> fallback, why's why // was added.

The fallback was only necessary if we wanted to combine the two
originally-identical loops to check all the required fields in one go,
before checking that specfied type is known.

But now that we're checking the type between the check for the common
and typ-specific fields that's no longer relevant.

Personally, as a long-time Perl programmer I prefer the conciseness of
the combined loop, but I guess in a predominantly C codebase it makes
sense to keep things simpler and more explicitly spelled out.

> v3 goes back to v1 plus I add a check for unknown field type per your
> suggestion, for example:
> ```
> error: guc_parameters.data line 2271: unknown GUC type 'intt'
> ```

We've already validated that the name field exists, so this error
message should include that, like the other ones do.

Otherwise, LGTM.

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