Re: multi-install PostgresNode fails with older postgres versions
Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
From: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
To: Andrew Dunstan <andrew@dunslane.net>
Cc: Mark Dilger <mark.dilger@enterprisedb.com>, PostgreSQL-development
<pgsql-hackers@postgresql.org>
Date: 2021-04-07T16:21:06Z
Lists: pgsql-hackers
On Wed, 7 Apr 2021 11:54:36 -0400 Andrew Dunstan <andrew@dunslane.net> wrote: > On 4/7/21 10:37 AM, Jehan-Guillaume de Rorthais wrote: > > Hi all, > > > > First, sorry to step in this discussion this late. I didn't noticed it > > before :( > > > > I did some work about these compatibility issues in late 2020 to use > > PostgresNode in the check_pgactivity TAP tests. > > > > See https://github.com/ioguix/check_pgactivity/tree/tests/t/lib > > > > PostgresNode.pm, TestLib.pm, SimpleTee.pm and RecursiveCopy.pm comes > > unchanged from PostgreSQL source file (see headers and COPYRIGHT.pgsql). > > > > Then, I'm using the facet class PostgresNodeFacet to extend it with some > > more methods. Finaly, I created one class per majpr version, each > > inheriting from the next version. That means 13 inherits from > > PostgresNodeFacet.pm, 12 inherits from 13, 11 inherits from 12, etc. > > > > When I'm creating a new node, I'm using the "pgaTester" factory class. It > > relies on PATH to check the major version using pg_config, then loads the > > appropriate class. > > > > That means some class overrides almost no methods but version(), which > > returns the major version. Eg.: > > https://github.com/ioguix/check_pgactivity/blob/tests/t/lib/PostgresNode12.pm > > > > From tests, I can check the node version using this method, eg.: > > > > skip "skip non-compatible test on PostgreSQL 8.0 and before", 3 > > unless $node->version <= 8.0; > > > > Of course, there's a lot of duplicate code between classes, but my main goal > > was to keep PostgresNode.pm unchanged from upstream so I can easily update > > it. > > > > And here is a demo test file: > > https://github.com/ioguix/check_pgactivity/blob/tests/t/01-streaming_delta.t > > > > My limited set of tests are working with versions back to 9.0 so far. > > > > My 2¢ > > > > I don't really want to create a multitude of classes. I think Mark is > basically on the right track. I started off using a subclass of > PostgresNode but was persuaded not to go down that route, and instead we > have made some fairly minimal changes to PostgresNode itself. I think > that was a good decision. If you take out the versioning subroutines, > the actual version-aware changes Mark is proposing to PostgresNode are > quite small - less that 200 lines supporting versions all the way back > to 8.1. That's pretty awesome. I took this road because as soon as you want to use some other methods like enable_streaming, enable_archiving, etc, you find much more incompatibilities on your way. My current stack of classes is backward compatible with much more methods than just init(). But I admit it creates a multitude of class and some duplicate code... It's still possible to patch each methods in PostgresNode, but you'll end up with a forest of conditional blocks depending on how far you want to go with old PostgreSQL versions. Regards,
Commits
-
Teach PostgresVersion all the ways to mark non-release code
- aa271209f6d9 14.0 landed
-
Make PostgresNode version aware
- 4c4eaf3d1920 14.0 landed
-
Avoid unfortunate IPC::Run path caching in PostgresNode
- 95c3a1956ec9 14.0 landed
-
Change pg_ctl to detect server-ready by watching status in postmaster.pid.
- f13ea95f9e47 10.0 cited