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



Reply via email to