Re: GUC flags

On Wed, Jan 26, 2022 at 03:29:29PM +0900, Michael Paquier wrote:
> On Tue, Jan 25, 2022 at 09:44:26PM -0600, Justin Pryzby wrote:
> > It seems like an arbitrary and short-sighted policy to expose a handful of
> > flags in the view for the purpose of retiring ./check_guc, but not expose other
> > flags, because we thought we knew that no user could ever want them.
> > 
> > We should either expose all the flags, or should put them into an undocumented
> > function.  Otherwise, how would we document the flags argument ?  "Shows some
> > of the flags" ?  An undocumented function avoids this issue.
> 
> My vote would be to have a documented function, with a minimal set of
> the flags exposed and documented, with the option to expand that in
> the future.  COMPUTED and EXPLAIN are useful, and allow some of the
> automated tests to happen.  NOT_IN_SAMPLE and GUC_NO_SHOW_ALL are less 
> useful for the user, and are more developer oriented, but are useful
> for the tests.  So having these four seem like a good first cut.

I implemented that (But my own preference would still be for an *undocumented*
function which returns whatever flags we find to be useful to include.  Or
alternately, a documented function which exposes every flag).

> +SELECT lower(name) FROM pg_settings_flags WHERE NOT not_in_sample EXCEPT
> +SELECT regexp_replace(ln, '^#?([_[:alpha:]]+) (= .*|[^ ]*$)', '\1') AS guc
> +FROM (SELECT regexp_split_to_table(pg_read_file('postgresql.conf'),
> '\n') AS ln) conf
> 
> Tests reading postgresql.conf would break on instances started with a
> custom config_file provided by a command line, no?

Maybe you misunderstood - I'm not reading the file specified by
current_setting('config_file').  Rather, I'm reading
tmp_check/data/postgresql.conf, which is copied from the sample conf.
Do you see an issue with that ?

The regression tests are only intended run from a postgres source dir, and if
someone runs the from somewhere else, and they "fail", I think that's because
they violated their assumption, not because of a problem with the test.

I wondered if it should chomp off anything added by pg_regress --temp-regress.
However that's either going to be a valid guc (or else it would fail some other
test).  Or an extention's guc (which this isn't testing), which has a dot, and
which this regex doesn't match, so doesn't cause false positives.

-- 
Justin