Thread

Re: Fix GUC_NO_SHOW_ALL test scenario in 003_check_guc.pl

Nitin Jadhav <nitinjadhavpostgres@gmail.com> — 2023-02-08T10:51:57Z

> Okay, the part to add an initialization check for GUC_NO_SHOW_ALL and
> GUC_NOT_IN_SAMPLE looked fine by me, so applied after more comment
> polishing.
>
> 0001 has been applied to clean up the existing situation.

Thanks for committing these 2 changes.


> On top of that, I have noticed an extra combination that would not
> make sense and that could be checked with the SQL queries:
> GUC_DISALLOW_IN_FILE implies GUC_NOT_IN_SAMPLE.  The opposite may not
> be true, though, as some developer GUCs are marked as
> GUC_NOT_IN_SAMPLE but they are allowed in a file.  The only exception
> to that currently is config_file.  It is just a special case whose
> value is enforced at startup and it can be passed down as an option
> switch via the postgres binary, still it seems like it would be better
> to also mark it as GUC_NOT_IN_SAMPLE?  This is done in 0002, only for
> HEAD, as that would be a new check.
>
> Remains
> 0002, that I am letting sleep to see if there's interest for it, or
> perhaps more ideas around it.

Makes sense and the patch looks good to me.

Thanks & Regards,
Nitin Jadhav

On Wed, Feb 8, 2023 at 1:29 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Mon, Feb 06, 2023 at 04:23:02PM +0900, Michael Paquier wrote:
> > On top of that, I have noticed an extra combination that would not
> > make sense and that could be checked with the SQL queries:
> > GUC_DISALLOW_IN_FILE implies GUC_NOT_IN_SAMPLE.  The opposite may not
> > be true, though, as some developer GUCs are marked as
> > GUC_NOT_IN_SAMPLE but they are allowed in a file.  The only exception
> > to that currently is config_file.  It is just a special case whose
> > value is enforced at startup and it can be passed down as an option
> > switch via the postgres binary, still it seems like it would be better
> > to also mark it as GUC_NOT_IN_SAMPLE?  This is done in 0002, only for
> > HEAD, as that would be a new check.
>
> 0001 has been applied to clean up the existing situation.  Remains
> 0002, that I am letting sleep to see if there's interest for it, or
> perhaps more ideas around it.
> --
> Michael