Re: multi-install PostgresNode fails with older postgres versions - Mailing list pgsql-hackers

> On Apr 7, 2021, at 1:28 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2021-Apr-07, Mark Dilger wrote:
>
>> I was commenting on the design to have the PostgresNode derived
>> subclass hard-coded to return "10" as the version:
>>
>>    sub version { return 10 }
>
> That seems a minor bug rather than a showstopper design deficiency.
> I agree that hardcoding the version in the source code is not very
> usable; it should store the version number when it runs pg_config
> --version in an instance variable that can be returned.

It seems we're debating between two designs.  In the first, each PostgresNode function knows about version limitations
andhas code like: 

    DoSomething() if $self->at_least_version("11")

and in the second design we're subclassing for each postgres release where something changed, so that DoSomething is
implementeddifferently in one class than another.  I think the subclassing solution is cleaner if the number of version
testsis large, but not so much otherwise. 


There is a much bigger design decision to be made that I have delayed making.  The PostgresNode implementation has
functionsthat work a certain way, but cannot work that same way with older versions of postgres that don't have the
necessarysupport.  This means that 

    $my_node->do_something(...)

works differently based on which version of postgres $my_node is based upon, even though PostgresNode could have
avoidedit.  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.
Priorto Andrew's recent introduction of support for alternate installation paths, it made sense to have
restart_after_crashbe off.  But now, if you spin up a postgres node for version 9.0 or before, you get different
behavior,because the prior behavior is to implicitly have this "on", not "off". 

Again:

    # "log_replication_commands" was introduced in 9.5.  Older versions do
    # not log replication commands.
    print $conf "log_replication_commands = on\n"
        if $self->at_least_version("9.5");

Should we have "log_replication_commands" be off by default so that nodes of varying postgres version behave more
similarly?

Again:

    # "wal_retrieve_retry_interval" was introduced in 9.5.  Older versions
    # always wait 5 seconds.
    print $conf "wal_retrieve_retry_interval = '500ms'\n"
        if $self->at_least_version("9.5");


Should we have "wal_retrieve_retry_interval" be 5 seconds for consistency?

I didn't do these things, as I didn't want to break the majority of tests which don't care about cross version
compatibility,but if we're going to debate this thing, subclassing is a distraction.  The real question is, *what do we
wantit to do*? 


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