Re: multi-install PostgresNode fails with older postgres versions

From: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Jehan-Guillaume de Rorthais <jgdr(at)dalibo(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: multi-install PostgresNode fails with older postgres versions
Date: 2021-04-07 21:34:41
Message-ID: 9A926D36-7B60-4485-A86D-CFA757E34D15@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Apr 7, 2021, at 2:04 PM, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>
> On 2021-Apr-07, Mark Dilger wrote:
>
>> It seems we're debating between two designs. In the first, each
>> PostgresNode function knows about version limitations and has code
>> like:
>>
>> DoSomething() if $self->at_least_version("11")
>
> Yeah, I didn't like this approach -- it is quite messy.
>
>> and in the second design we're subclassing for each postgres release
>> where something changed, so that DoSomething is implemented
>> differently in one class than another.
>
> So DoSomething still does Something, regardless of what it has to do in
> order to get it done.
>
>> I think the subclassing solution is cleaner if the number of version
>> tests is large, but not so much otherwise.
>
> Well, your patch has rather a lot of at_least_version() tests.

I don't really care about this part, and you do, so you win. I'm happy enough to have this be done with subclassing. My problems upthread never had anything to do with whether we used subclassing, but rather which behaviors were supported. And that appears not to be controversial, so that's all for the good....

>> To wit:
>>
>> # "restart_after_crash" was introduced in version 9.1. Older versions
>> # always restart after crash.
>> print $conf "restart_after_crash = off\n"
>> if $self->at_least_version("9.1");
>>
>> PostgresNode is mostly designed around supporting regression tests for
>> the current postgres version under development.
>
> I think we should make PostgresNode do what makes the most sense for the
> current branch, and go to whatever contortions are necessary to do the
> same thing in older versions as long as it is sensible. If we were
> robots, then we would care to preserve behavior down to the very last
> byte, but I think we can make judgement calls to ignore the changes that
> are not relevant. Whenever we introduce a behavior that is not
> supportable by the older version, then the function would throw an error
> if that behavior is requested from that older version.

Beep bop boop beeb bop, danger Will Robinson:

for my $i (@all_postgres_versions)
{
for my $j (grep { $_ > $i } @all_postgres_versions)
{
for my $k (grep { $_ > $j } @all_postgres_versions)
{
my $node = node_of_version($i);
$node->do_stuff();
$node->pg_upgrade_to($j);
$node->do_more_stuff();
$node->pg_upgrade_to($k);
$node->do_yet_more_stuff();

# verify $node isn't broken
}
}
}

I think it is harder to write simple tests like this when how $node behaves changes as the values of (i,j,k) change. Of course the behaviors change to the extent that postgres itself changed between versions. I mean changes because PostgresNode behaves differently.

We don't need to debate this now, though. It will be better to discuss individual issues as they come up.


Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2021-04-07 21:39:39 Re: psql \df choose functions by their arguments
Previous Message Greg Sabino Mullane 2021-04-07 21:25:13 Re: psql \df choose functions by their arguments