Re: Fix GUC_NO_SHOW_ALL test scenario in 003_check_guc.pl

From: Nitin Jadhav <nitinjadhavpostgres(at)gmail(dot)com>
To: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Fix GUC_NO_SHOW_ALL test scenario in 003_check_guc.pl
Date: 2023-01-14 13:40:55
Message-ID: CAMm1aWb3fLesDPX+M0LXr_xee71H2k=cEz2v7A3GBiB=5d+kzA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> Looks like you're right ; show_all_settings() elides settings marked
> "noshow".
>
> Do you know how you'd implement a fix ?

I could think of the following options.

Option-1 is, expose a function like pg_settings_get_no_show_all()
which just returns the parameters which are just listed as
GUC_NO_SHOW_ALL (Not in combination with NOT_IN_SAMPLE). We can then
use this function in the test file and verify whether there are config
entries for these.

Option-2 is, if exposing new function and that too to expose
parameters which are listed as GUC_NO_SHOW_ALL is not recommended,
then how about exposing a function like pg_settings_get_count() which
returns the count of all parameters including GUC_NO_SHOW_ALL. We can
then use this number to verify whether these many are present in the
sample config file. But we cannot show the name of the parameters if
it is not matching. We can just display an error saying "Parameter
with GUC_NO_SHOW_ALL is missing from postgresql.conf.sample".

Option-3 is, if exposing both of the above functions is not
recommended, then we can use the existing function
pg_settings_get_flags() for each of the parameters while reading the
sample config file in 003_check_guc.pl. This validates the
GUC_NO_SHOW_ALL parameter if that is present in the sample config
file. It does not validate if it is present in guc.c and missing in
the sample config file.

Option-4 is, how about manually adding the parameter name to
'all_params_array' in 003_check_guc.pl whenever we add such GUCs.

I am not able to choose any of the above options as each has some
disadvantages but if no other options exist, then I would like to go
with option-3 as it validates more than the one currently doing.
Please share if any other better ideas.

Thanks & Regards,
Nitin Jadhav

On Fri, Jan 13, 2023 at 7:32 PM Justin Pryzby <pryzby(at)telsasoft(dot)com> wrote:
>
> On Fri, Jan 13, 2023 at 06:15:38PM +0530, Nitin Jadhav wrote:
> > Hi,
> >
> > The commit 7265dbffad7feac6ea9d373828583b5d3d152e07 has added a script
> > in src/backend/utils/misc/check_guc that cross-checks the consistency
> > of the GUCs with postgresql.conf.sample, making sure that its format
> > is in line with what guc.c has. As per the commit message, the
> > parameters which are not listed as NOT_IN_SAMPLE in guc.c should be
> > present in postgresql.conf.sample. But I have observed a test case
> > failure when the parameters which are listed as GUC_NO_SHOW_ALL in
> > guc.c and if it is present in postgresql.conf.sample. I feel this
> > behaviour is not expected and this should be fixed. I spent some time
> > on the analysis and found that query [1] is used to fetch all the
> > parameters which are not listed as NOT_IN_SAMPLE. But the pg_settings
> > view does not return the parameters listed as GUC_NO_SHOW_ALL. Hence
> > these records will be missed. Please share your thoughts. I would like
> > to work on the patch if a fix is required.
>
> Looks like you're right ; show_all_settings() elides settings marked
> "noshow".
>
> Do you know how you'd implement a fix ?
>
> --
> Justin

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Gurjeet Singh 2023-01-14 14:14:02 Re: Named Operators
Previous Message Nikita Malakhov 2023-01-14 11:47:57 Re: Pluggable toaster