[PATCH] We install pg_regress and isolationtester but not pg_isolation_regress

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] We install pg_regress and isolationtester but not pg_isolation_regress

Craig Ringer-3
Hi

Per topic, the Pg makefiles install pg_regress (for use by extensions) and htey install the isolationtester, but they don't install pg_isolation_regress.

We should install it too.

--
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

0001-Install-pg_isolation_regress.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] We install pg_regress and isolationtester but not pg_isolation_regress

Craig Ringer-3
On 28 May 2018 at 15:06, Craig Ringer <[hidden email]> wrote:
Hi

Per topic, the Pg makefiles install pg_regress (for use by extensions) and htey install the isolationtester, but they don't install pg_isolation_regress.

We should install it too.

Now with a patch that isn't brain-dead.

I'm wondering if I should add ISOLATION support to PGXS too, like we have REGRESS .

--
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

v2-0001-Install-pg_isolation_regress-not-just-isolationte.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] We install pg_regress and isolationtester but not pg_isolation_regress

Alvaro Herrera-9
On 2018-Jun-01, Craig Ringer wrote:

> On 28 May 2018 at 15:06, Craig Ringer <[hidden email]> wrote:
>
> > Per topic, the Pg makefiles install pg_regress (for use by extensions) and
> > htey install the isolationtester, but they don't install
> > pg_isolation_regress.
> >
> > We should install it too.

I happened to come across this thread by accident, and I tend to agree
that we need to install both isolationtester and pg_isolation_regress to
the pgxs dirs, just like we do pg_regress.  I can't find that
isolationtester is installed anywhere though; maybe that changed after
you posted this.  Anyway, this version of the patch installs both.

I did search for evidence in the Makefile's git log that would remove a
recipe for installing isolationtester; couldn't find anything.

> I'm wondering if I should add ISOLATION support to PGXS too, like we have
> REGRESS .

This was covered by Michael's d3c09b9b1307 a few months after you
posted.

--
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

v3-0001-Install-pg_isolation_regress-and-isolationtester.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] We install pg_regress and isolationtester but not pg_isolation_regress

Craig Ringer-3

On Tue, 29 Sep 2020 at 22:09, Alvaro Herrera <[hidden email]> wrote:
On 2018-Jun-01, Craig Ringer wrote:

> On 28 May 2018 at 15:06, Craig Ringer <[hidden email]> wrote:
>
> > Per topic, the Pg makefiles install pg_regress (for use by extensions) and
> > htey install the isolationtester, but they don't install
> > pg_isolation_regress.
> >
> > We should install it too.

I happened to come across this thread by accident, and I tend to agree
that we need to install both isolationtester and pg_isolation_regress to
the pgxs dirs, just like we do pg_regress.  I can't find that
isolationtester is installed anywhere though; maybe that changed after
you posted this.  Anyway, this version of the patch installs both.

Thanks.

I think rules were added to allow the isolation tester to be installed with an explicit make -C src/test/isolation install, but not added to the default "install" target.

I agree it should just be installed.

--
 Craig Ringer                   http://www.2ndQuadrant.com/
 2ndQuadrant - PostgreSQL Solutions for the Enterprise
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] We install pg_regress and isolationtester but not pg_isolation_regress

Álvaro Herrera
On 2020-Sep-30, Craig Ringer wrote:

> On Tue, 29 Sep 2020 at 22:09, Alvaro Herrera <[hidden email]>
> wrote:

> > I happened to come across this thread by accident, and I tend to agree
> > that we need to install both isolationtester and pg_isolation_regress to
> > the pgxs dirs, just like we do pg_regress.  I can't find that
> > isolationtester is installed anywhere though; maybe that changed after
> > you posted this.  Anyway, this version of the patch installs both.

> I think rules were added to allow the isolation tester to be installed with
> an explicit make -C src/test/isolation install, but not added to the
> default "install" target.
>
> I agree it should just be installed.

Pushed, thanks.


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] We install pg_regress and isolationtester but not pg_isolation_regress

Álvaro Herrera
On 2020-Oct-15, Alvaro Herrera wrote:

> Pushed, thanks.

I forgot to mention that I considered backpatching this and decided not
to, but only because it might confuse packagers if they see unrecognized
files in the installation.  I realize now that c3a0818460a8 was
back-patched.  Any opinions on backpatching?  


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] We install pg_regress and isolationtester but not pg_isolation_regress

Tom Lane-2
Alvaro Herrera <[hidden email]> writes:
> I forgot to mention that I considered backpatching this and decided not
> to, but only because it might confuse packagers if they see unrecognized
> files in the installation.  I realize now that c3a0818460a8 was
> back-patched.  Any opinions on backpatching?  

We've added new installed files in minor releases before, true.
But I agree it's something to do only when pretty important, and I'm
not sure this clears the bar.  TAP tests (the facility added by that
other patch) seem way more commonly useful than isolation tests.

Quickly counting the uses in our existing in-core extensions, I see

TAP_TESTS: 3 contrib, 5 src/test/modules
ISOLATION: 1 contrib, 3 src/test/modules

Other than src/test/modules/brin, the ISOLATION users don't look
much like real extensions (rather than test scaffolding), either.
If you discount test scaffolding modules then the use-counts are
more like 4 to 1.

So I'm -0.1 or so on backpatching.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] We install pg_regress and isolationtester but not pg_isolation_regress

Michael Paquier-2
On Thu, Oct 15, 2020 at 01:06:54PM -0400, Tom Lane wrote:
> Other than src/test/modules/brin, the ISOLATION users don't look
> much like real extensions (rather than test scaffolding), either.
> If you discount test scaffolding modules then the use-counts are
> more like 4 to 1.

Out of core, the only thing I can see with isolation tests is rum, but
it uses a workaround to have an access to the necessary binaries.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] We install pg_regress and isolationtester but not pg_isolation_regress

Craig Ringer-5


On Fri, 16 Oct 2020, 09:00 Michael Paquier, <[hidden email]> wrote:
On Thu, Oct 15, 2020 at 01:06:54PM -0400, Tom Lane wrote:
> Other than src/test/modules/brin, the ISOLATION users don't look
> much like real extensions (rather than test scaffolding), either.
> If you discount test scaffolding modules then the use-counts are
> more like 4 to 1.

Out of core, the only thing I can see with isolation tests is rum, but
it uses a workaround to have an access to the necessary binaries.

I would've liked to backpatch but don't really care very much. If it's going to take time away from others things, don't do it. 

I landed up having to make my own lightly customised postgres packages to use as test workflow inputs anyway. So I included the full set of isolation test utilities, packaged the test inputs etc.

I'd prefer not to have to do it, but it's done. So long as it's fixed going forward it didn't matter that much.

Now server_version_num on the other hand ... :P