Re: Fix GUC_NO_SHOW_ALL test scenario in 003_check_guc.pl - Mailing list pgsql-hackers
From | Nitin Jadhav |
---|---|
Subject | Re: Fix GUC_NO_SHOW_ALL test scenario in 003_check_guc.pl |
Date | |
Msg-id | CAMm1aWb3fLesDPX+M0LXr_xee71H2k=cEz2v7A3GBiB=5d+kzA@mail.gmail.com Whole thread Raw |
In response to | Re: Fix GUC_NO_SHOW_ALL test scenario in 003_check_guc.pl (Justin Pryzby <pryzby@telsasoft.com>) |
Responses |
Re: Fix GUC_NO_SHOW_ALL test scenario in 003_check_guc.pl
|
List | 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@telsasoft.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
pgsql-hackers by date: