> On Apr 7, 2021, at 11:35 AM, Jehan-Guillaume de Rorthais <jgdr@dalibo.com> wrote:
>
> On Wed, 7 Apr 2021 13:38:39 -0400
> Andrew Dunstan <andrew@dunslane.net> wrote:
>
>> 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.
>
> I started with a patched PostgresNode. Then I had to support backups and
> replication for my tests. Then it become hard to follow the logic in
> conditional blocks, moreover some conditionals needed to appear in multiple
> places in the same methods, depending on the enabled features, the conditions,
> what GUC was enabled by default or not, etc. So I end up with this design.
>
> I really don't want to waste community brain cycles in discussions and useless
> reviews. But as far as someone agree to review it, I already have the material
> and I can create a patch with a limited amount of work to compare and review.
> The one-class approach would need to support replication down to 9.0 to be fair
> though :/
If you can create a patch that integrates your ideas into PostgresNode.pm, I'd be interested in reviewing it.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
