Re: Fix GUC_NO_SHOW_ALL test scenario in 003_check_guc.pl

On Sat, Jan 14, 2023 at 07:10:55PM +0530, Nitin Jadhav wrote:
> 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".

We would miss the names of the parameters that are marked as NO_SHOW,
missing from pg_settings, making debugging harder.

> 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.

This would make the test more costly by forcing one SQL for each
GUC..

> 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.

We could extend pg_show_all_settings() with a boolean parameter to
enforce listing all the parameters, even the ones that are marked as
NOSHOW, but this does not count on GetConfigOptionValues() that could
force a parameter to become noshow on a superuser-only GUC depending
on the role that's running the function. At the end, we'd better rely
on a separate superuser-only function to do this job, aka option 1.

How much do we need to care with the duplication this would involve
with show_all_settings(), actually? If you don't use the SRF macros,
the code would just be a couple of lines with InitMaterializedSRF()
doing a loop on GetConfigOptionValues(). Even if that means listing
twice the parameters in pg_proc.dat, the chances of adding new
parameters in pg_settings is rather low so that would be a one-time
change?
--
Michael