On 14/02/2023 03:42, Kyotaro Horiguchi wrote:
At Mon, 13 Feb 2023 12:18:07 +0900, Michael Paquier <mich...@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



Reply via email to