Re: GUC flags
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, pgsql-hackers@lists.postgresql.org, bruce@momjian.us, tgl@sss.pgh.pa.us
Date: 2022-01-29T06:38:53Z
Lists: pgsql-hackers
Attachments
- v2-0001-Expose-GUC-flags-in-SQL-function-replacing-check_.patch (text/x-diff) patch v2-0001
On Thu, Jan 27, 2022 at 10:36:21PM -0600, Justin Pryzby wrote:
> 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 ?
Yes, as of:
mv $PGDATA/postgresql.conf $PGDATA/popo.conf
pg_ctl start -D $PGDATA -o '-c config_file=popo.conf'
make installcheck
I have not checked, but I am pretty sure that a couple of
distributions out there would pass down a custom path for the
configuration file, while removing postgresql.conf from the data
directory to avoid any confusion because if one finds out that some
parameters are defined but not loaded. Your patch fails on 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.
The tests are able to work out on HEAD, I'd rather not break something
that has worked this way for years. Two other aspects that we may
want to worry about are include_dir and include if we were to add
tests for that, perhaps. This last part is not really a strong
requirement IMO, though.
> 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.
I am not sure about those parts, being reserved about the parts that
involve the format of postgresql.conf or any other configuration
parts, but we could tackle that after, if necessary.
For now, I have down a review of the patch, tweaking the docs, the
code and the test to take care of all the inconsistencies I could
find. This looks like a good first cut to be able to remove check_guc
(the attached removes it, but I think that we'd better treat that
independently of the actual feature proposed, for clarity).
--
Michael
Commits
-
Add TAP test to automate the equivalent of check_guc, take two
- 7265dbffad7f 15.0 landed
-
Retire src/backend/utils/misc/check_guc
- cf29a11ef646 15.0 landed
-
Add TAP test to automate the equivalent of check_guc
- b0a55f4d4ad5 15.0 landed
-
Add PostgreSQL::Test::Cluster::config_data()
- ba15f16107be 15.0 landed
-
Introduce pg_settings_get_flags() to find flags associated to a GUC
- d10e41d4238e 15.0 landed
-
Revert changes about warnings/errors for placeholders.
- cab5b9ab2c06 15.0 cited
-
Improve the description of various GUCs
- 03774f9bb304 15.0 landed