|From:||Andres Freund <andres(at)anarazel(dot)de>|
|To:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>|
|Cc:||Andrew Dunstan <andrew(at)dunslane(dot)net>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, pgsql-hackers(at)postgresql(dot)org|
|Subject:||Re: Handle TEMP_CONFIG for pg_regress style tests in pg_regress.c|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
On 2023-02-19 11:13:38 -0500, Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> > On 2023-02-19 Su 02:25, Peter Eisentraut wrote:
> >> On 18.02.23 21:26, Andres Freund wrote:
> >>> My inclination is to move TEMP_CONFIG support from the Makefile to
> >>> pg_regress.c. That way it's consistent across the build tools and isn't
> >>> duplicated.
> >> I'm having a hard time understanding what TEMP_CONFIG is for.
> > It's used by the buildfarm to add the extra config settings from its
> > configuration file.
> I have also used it manually to inject configuration changes into
> TAP tests, for instance running them with debug_discard_caches = 1.
Similar. Explicitly turning on fsync, changing the log level to debug etc.
> It's quite handy, but I agree the lack of documentation is bad.
We have some minimal documentation for EXTRA_REGRESS_OPTS, but imo that's a
bit of a different use case, as it adds actual commandline options for
pg_regress (and thus doesn't work for tap tests).
Seems we'd need a section in regress.sgml documenting the various environment
> It looks to me like pg_regress already does implement this; that
> is, the Makefiles convert TEMP_CONFIG into a --temp-config switch
> to pg_[isolation_]regress. So if we made pg_regress responsible
> for examining the envvar directly, very little new code would be
It's very little, indeed - the patch upthread ends up with:
4 files changed, 11 insertions(+), 16 deletions(-)
> (Maybe net negative code if we remove the command line
> switch, but I'm not sure if we should.)
I don't think we should - we use it for various regression tests, to specify a
config file they should load (shared_preload_libraries, wal_level, etc).
The way I implemented it now is that TEMP_CONFIG is added earlier in the
resulting config file, than the contents of the file explicitly specified on
> What we'd lose is the ability to write make TEMP_CONFIG=foo check but I
> wouldn't miss that. Having a uniform rule that TEMP_CONFIG is an
> environment variable and nothing else seems good.
If we were concerned about it we could just add an export of TEMP_CONFIG to
|Next Message||Tom Lane||2023-02-19 20:56:22||Re: Wrong query results caused by loss of join quals|
|Previous Message||Joel Jacobson||2023-02-19 19:54:11||Re: Missing free_var() at end of accum_sum_final()?|