Re: [PATCH] Fix pg_upgrade test from v10

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Michael Paquier <michael(at)paquier(dot)xyz>, "Anton A(dot) Melnikov" <aamelnikov(at)inbox(dot)ru>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [PATCH] Fix pg_upgrade test from v10
Date: 2022-06-02 20:57:12
Message-ID: 1b2174ed-74ec-d02f-bded-c61c041c0156@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On 2022-06-01 We 21:37, Michael Paquier wrote:
> On Thu, Jun 02, 2022 at 04:22:52AM +0300, Anton A. Melnikov wrote:
>> Found out that test for pg_upgrade (test.sh for 11-14 and 002_pg_upgrade.pl
>> for 15+) doesn't work from 10th versions to higher ones due to incompatible
>> options for initdb and default PGDATA permissions.
> Yeah, there are still TODOs in this stuff. Those tests can also break
> easily depending on the dump you are pushing to the old node when
> doing cross-version upgrades. Perl makes it a bit easier to reason
> about improving this area in the future, though, and MSVC is able to
> catch up on that.
>
>> # To increase coverage of non-standard segment size and group access without
>> # increasing test runtime, run these tests with a custom setting.
>> # --allow-group-access and --wal-segsize have been added in v11.
>> -$oldnode->init(extra => [ '--wal-segsize', '1', '--allow-group-access' ]);
>> +my ($oldverstr) = `$ENV{oldinstall}/bin/pg_ctl --version` =~ /(\d+\.\d+)/;
>> +my ($oldver) = (version->parse(${oldverstr}));
>> +$oldnode->init(extra => [ '--wal-segsize', '1', '--allow-group-access' ])
>> + if $oldver >= version->parse('11.0');
>> +$oldnode->init()
>> + if $oldver < version->parse('11.0');
> A node's pg_version is assigned via _set_pg_version() when creating it
> using PostgreSQL::Test::Cluster::new(). In order to make the
> difference with the set of initdb options to use when setting up the
> old node, it would be simpler to rely on that, no? Version.pm is able
> to handle integer as well as string comparisons for the version
> strings.

Both these patches look dubious.

1. There is no mention of why there's a change w.r.t. Cygwin and 
permissions checks. Maybe it's ok, but it seems off topic and is
certainly not referred to in the patch submission.

2. As Michael says, we should not be using perl's version module, we
should be using the version object built into each
PostgreSQL::Test::Cluster instance.

cheers

andrew

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2022-06-02 21:06:18 Re: replacing role-level NOINHERIT with a grant-level option
Previous Message Robert Haas 2022-06-02 20:44:02 Re: pg_auth_members.grantor is bunk