Re: Fix GUC_NO_SHOW_ALL test scenario in 003_check_guc.pl

On 14/02/2023 03:42, Kyotaro Horiguchi wrote:
> At Mon, 13 Feb 2023 12:18:07 +0900, Michael Paquier <michael@paquier.xyz> wrote in
>> On Mon, Feb 13, 2023 at 11:27:58AM +0900, Kyotaro Horiguchi wrote:
>>> I think currently the output by --describe-config can be used only for
>>> consulting while editing a (possiblly broken) config file.  Thus I
>>> think it's no use showing GIC_DISALLOW_IN_FILE items there unless we
>>> use help_config() for an on-session use.
>>>
>>> On the other hand, don't we need to remove the condition
>>> GUC_NOT_IN_SAMPLE from displayStruct? I think that help_config()
>>> should show a value if it is marked as !GUC_DISALLOW_IN_FILE even if
>>> it is GUC_NOT_IN_SAMPLE.  I'm not sure whether there's any variable
>>> that are marked that way, though.
>>
>> As in marked with GUC_NOT_IN_SAMPLE but not GUC_DISALLOW_IN_FILE?
>> There are quite a lot, developer GUCs being one (say
>> ignore_invalid_pages).  We don't want to list them in the sample file
>> so as common users don't play with them, still they make sense if
>> listed in a file.
> 
> Ah, right.  I think I faintly had them in my mind.
> 
>> If you add a check meaning that GUC_DISALLOW_IN_FILE implies
>> GUC_NOT_IN_SAMPLE, where one change would need to be applied to
>> config_file as all the other GUC_DISALLOW_IN_FILE GUCs already do
>> that, you could remove GUC_DISALLOW_IN_FILE.  However,
>> GUC_NOT_IN_SAMPLE should be around to not expose options, we don't
>> want common users to know too much about.
> 
> Okay, I thought that "postgres --help-config" was a sort of developer
> option, but your explanation above makes sense.
> 
>> The question about how much people rely on --describe-config these
>> days is a good one, so perhaps there could be an argument in removing
> 
> Yeah, that the reason for my thought it was a developer option...
> 
>> GUC_NOT_IN_SAMPLE from the set.  TBH, I would be really surprised that
>> anybody able to use a developer option writes an configuration file in
>> an incorrect format and needs to use this option, though :)
> 
> Hmm.  I didn't directly link GUC_NOT_IN_SAMPLE to being a developer
> option. But on second thought, it seems that it is. So, the current
> code looks good for me now. Thanks for the explanation.

The first patch was committed, and there's not much enthusiasm for 
disallowing (GUC_DISALLOW_IN_FILE && !GUC_NOT_IN_SAMPLE), so I am 
marking this as Committed in the commitfest app. Thanks!

- Heikki