Re: multi-install PostgresNode fails with older postgres versions

Andrew Dunstan <andrew@dunslane.net>

From: Andrew Dunstan <andrew@dunslane.net>
To: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Mark Dilger <mark.dilger@enterprisedb.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2021-04-07T17:38:39Z
Lists: pgsql-hackers
On 4/7/21 1:19 PM, Jehan-Guillaume de Rorthais wrote:
> On Wed, 7 Apr 2021 12:51:55 -0400
> Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
>> On 2021-Apr-07, Jehan-Guillaume de Rorthais wrote:
>>
>>> 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.  
>> From a code cleanliness point of view, I agree that having separate
>> classes for each version is neater than what you call a forest of
>> conditionals.  I'm not sure I like the way you instantiate the classes
>> in pgaTester though -- wouldn't it be saner to have PostgresNode::new
>> itself be in charge of deciding which class to bless the object as?
>> Since we're talking about modifying PostgresNode itself in order to
>> support this, it would make sense to do that.
> Yes, it would be much saner to make PostgresNode the factory class. Plus, some
> more logic could be injected there to either auto-detect the version (current
> behavior) or eg. use a given path to the binaries as Mark did in its patch.


Aren't you likely to end up duplicating substantial amounts of code,
though? I'm certainly not at the stage where I think the version-aware
code is creating too much clutter. The "forest of conditionals" seems
more like a small thicket.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com




Commits

  1. Teach PostgresVersion all the ways to mark non-release code

  2. Make PostgresNode version aware

  3. Avoid unfortunate IPC::Run path caching in PostgresNode

  4. Change pg_ctl to detect server-ready by watching status in postmaster.pid.