From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Justin Pryzby <pryzby(at)telsasoft(dot)com> |
Cc: | Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, bruce(at)momjian(dot)us, tgl(at)sss(dot)pgh(dot)pa(dot)us |
Subject: | Re: GUC flags |
Date: | 2022-01-29 06:38:53 |
Message-ID: | YfTg/WHNLVVygy8v@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
v2-0001-Expose-GUC-flags-in-SQL-function-replacing-check_.patch | text/x-diff | 12.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Julien Rouhaud | 2022-01-29 07:51:33 | Re: Multiple Query IDs for a rewritten parse tree |
Previous Message | Michael Paquier | 2022-01-29 05:24:24 | Re: make MaxBackends available in _PG_init |