Re: GUC flags

On Sat, Jan 29, 2022 at 06:18:50PM -0600, Justin Pryzby wrote:
> "The most meaningful ones are included" doesn't seem to add anything.
> Maybe it'd be useful to say "(Only the most useful flags are exposed)"

Yes, I have used something like that.

> I think the description is wrong, or just copied from the others.
> EXPLAIN is for GUCs which are shown in EXPLAIN(SETTINGS).

Added some details here.

> |EXPLAIN, parameters included in EXPLAIN commands.
> |NO_SHOW_ALL, parameters excluded from SHOW ALL commands.
> |NO_RESET_ALL, parameters excluded from RESET ALL commands.
> |NOT_IN_SAMPLE, parameters not included in postgresql.conf by default.
> |RUNTIME_COMPUTED, runtime-computed parameters.
>
> Instead of a comma, these should use a colon, or something else?

And switched to a colon here.

With all those doc fixes, applied after an extra round of review. So
this makes us rather covered with the checks on the flags.

Now, what do we do with the rest of check_guc that involve a direct
lookup at what's on disk. We have the following:
1) Check the format of the option lists in guc.c.
2) Check the format of postgresql.conf.sample:
-- Valid options preceded by a '#' character.
-- Valid options followed by ' =', with at least one space before the
equal sign.
3) Check that options not marked as NOT_IN_SAMPLE are in the sample
file.

I have never seen 1) as a problem, and pgindent takes care of that at
some degree. 2) is also mostly cosmetic, and committers are usually
careful when adding a new GUC. 3) would be the most interesting
piece, and would cover most cases if we consider that a default
installation just copies postgresql.conf.sample over, as proposed
upthread in 0002.

Now, 3) has also the problem that it would fail installcheck as one
can freely add a developer option in the configuration. We could
solve that by adding a check in a TAP test, by using pg_config
--sharedir to find where the sample file is located. I wonder if this
would be a problem for some distributions, though, so adding such a
dependency feels a bit scary even if it would mean that initdb is
patched.

As a whole, I'd like to think that we would not lose much if check_guc
is removed.
--
Michael