PATCH pass PGOPTIONS to pg_regress

Started by Manuel Kniepover 7 years ago5 messages
#1Manuel Kniep
m.kniep@web.de
1 attachment(s)

Hi,

attached patch passes PGOPTIONS env variable to pg_regress in pgxs.mk

This is especially useful when developing extensions for different postgres versions
where some session_variables might or might not exists.

Consider something like this in an extensions makefile:

ifeq ($(shell test $(VERSION_NUM) -ge 90600; echo $$?),0)
PGOPTIONS+= "--max_parallel_workers_per_gather=0"
endif

But also when there are many testfiles it might be convenient to align some session parameter globally
In the Makefile.

Thoughts?

Cheers

Manuel Kniep

Attachments:

0001-pg_regress-pass-pgoptions.patchapplication/octet-stream; name=0001-pg_regress-pass-pgoptions.patch; x-unix-mode=0644Download
diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk
index bfae02f90c..6d75ecc0a8 100644
--- a/src/makefiles/pgxs.mk
+++ b/src/makefiles/pgxs.mk
@@ -299,7 +299,7 @@ endif
 # against installed postmaster
 ifndef NO_INSTALLCHECK
 installcheck: submake $(REGRESS_PREP)
-	$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)
+	PGOPTIONS=$(PGOPTIONS) $(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)
 endif
 
 ifdef PGXS
@@ -308,7 +308,7 @@ check:
 	@echo 'Do "$(MAKE) install", then "$(MAKE) installcheck" instead.'
 else
 check: submake $(REGRESS_PREP)
-	$(pg_regress_check) $(REGRESS_OPTS) $(REGRESS)
+	PGOPTIONS=$(PGOPTIONS) $(pg_regress_check) $(REGRESS_OPTS) $(REGRESS)
 
 temp-install: EXTRA_INSTALL+=$(subdir)
 endif
#2Ildar Musin
i.musin@postgrespro.ru
In reply to: Manuel Kniep (#1)
Re: PATCH pass PGOPTIONS to pg_regress

Hi Manuel,

On 29.05.2018 16:19, Manuel Kniep wrote:

Hi,

attached patch passes PGOPTIONS env variable to pg_regress in
pgxs.mk

This is especially useful when developing extensions for different
postgres versions where some session_variables might or might not
exists.

Consider something like this in an extensions makefile:

ifeq ($(shell test $(VERSION_NUM) -ge 90600; echo $$?),0) PGOPTIONS+=
"--max_parallel_workers_per_gather=0" endif

But also when there are many testfiles it might be convenient to
align some session parameter globally In the Makefile.

Have you considered using EXTRA_REGRESS_OPTS variable in extension's
Makefile?

EXTRA_REGRESS_OPTS=--temp-config=$(top_srcdir)/$(subdir)/extra.conf

Here extra.conf is implied to be a file in extension's root directory
which contains additional server options.
This would only work for `make check` though, not `make installcheck`.

--
Ildar Musin
i.musin@postgrespro.ru

#3Michael Paquier
michael@paquier.xyz
In reply to: Ildar Musin (#2)
Re: PATCH pass PGOPTIONS to pg_regress

On Wed, May 30, 2018 at 12:28:27PM +0300, Ildar Musin wrote:

Here extra.conf is implied to be a file in extension's root directory which
contains additional server options.
This would only work for `make check` though, not `make installcheck`.

REGRESS_OPTS is more widely known for this purpose, and I use it as
well. Still, I agree that there is no need to add an extra mechanism
with PGOPTIONS if a feature already exists. One thing which is also
not much known is that multiple --temp-config entries can be defined
where the last entry wins if the same parameter maps across multiple
files, so you can reduce configuration delta chunks if you need to worry
about multiple versions of the backend for the same development branch

I tend to prefer using one branch per stable version to map with
Postgres and it makes code chunks easier to see without thinking about
PG_VERSION_NUM, still that depends on the project developped.
--
Michael

#4Manuel Kniep
rapimo@adeven.com
In reply to: Michael Paquier (#3)
Re: PATCH pass PGOPTIONS to pg_regress

Am 30.05.2018 um 17:22 schrieb Michael Paquier <michael@paquier.xyz>:

On Wed, May 30, 2018 at 12:28:27PM +0300, Ildar Musin wrote:

Here extra.conf is implied to be a file in extension's root directory which
contains additional server options.
This would only work for `make check` though, not `make installcheck`.

REGRESS_OPTS is more widely known for this purpose, and I use it as
well. Still, I agree that there is no need to add an extra mechanism
with PGOPTIONS if a feature already exists. One thing which is also

--temp-config is only supported in temp-instance mode which I wanted to avoid

But if you think that this feature is supported enough by this I agree it doesn’t make sense
to add something. I just don’t see any disadvantage in my approach.

Manuel

#5Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Manuel Kniep (#1)
Re: PATCH pass PGOPTIONS to pg_regress

On 5/29/18 09:19, Manuel Kniep wrote:

Consider something like this in an extensions makefile:

ifeq ($(shell test $(VERSION_NUM) -ge 90600; echo $$?),0)
PGOPTIONS+= "--max_parallel_workers_per_gather=0"
endif

I think you can probably write this inside your test .sql file using a
bit of PL/pgSQL.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services