Re: Extracting cross-version-upgrade knowledge from buildfarm client

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Extracting cross-version-upgrade knowledge from buildfarm client
Date: 2023-07-19 20:47:28
Message-ID: dc0f4da5-4f18-92d0-771d-de97f300c449@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On 2023-07-19 We 16:44, Andrew Dunstan wrote:
>
>
> On 2023-07-19 We 15:20, Andrew Dunstan wrote:
>>
>>
>> On 2023-07-19 We 12:05, Alvaro Herrera wrote:
>>
>>
>>>> Maybe we need to make AdjustUpgrade just look at the major version,
>>>> something like:
>>>>
>>>>    $old_version = PostgreSQL::Version->new($old_version->major);
>>> It seems like that does work, but if we do that, then we also need to
>>> change this line:
>>>
>>> if ($old_version lt '9.5')
>>> to
>>> if ($old_version < '9.5')
>>>
>>> otherwise you get some really mysterious failures about trying to drop
>>> public.=>, which is in fact no longer accepted syntax since 9.5; and the
>>> stringwise comparison returns the wrong value here.
>>
>>
>> That seems odd. String comparison like that is supposed to work. I
>> will do some tests.
>>
>>
>>> TBH I'm getting a sense of discomfort with the idea of having developed
>>> a Postgres-version-number Perl module, and in the only place where we
>>> can use it, have to settle for numeric comparison instead.
>>
>>
>> These comparisons only look like that. They are overloaded in
>> PostgreSQL::Version.
>>
>
> The result you report suggest to me that somehow the old version is no
> longer a PostgreSQL::Version object.  Here's the patch I suggest:
>
>
> diff --git a/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm
> b/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm
> index a241d2ceff..d7a7383deb 100644
> --- a/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm
> +++ b/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm
> @@ -74,6 +74,11 @@ values are arrayrefs to lists of statements to be
> run in those databases.
>  sub adjust_database_contents
>  {
>     my ($old_version, %dbnames) = @_;
> +
> +   die "wrong type for \$old_version\n"
> +     unless $old_version->isa("PostgreSQL::Version");
> +   $old_version = PostgreSQL::Version->new($old_version->major);
> +
>     my $result = {};
>
>     # remove dbs of modules known to cause pg_upgrade to fail
>
>
> Do you still see errors with that?
>
>
>

Just realized it would need to be applied in all three exported routines.

cheers

andrew

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2023-07-19 21:01:04 Re: logical decoding and replication of sequences, take 2
Previous Message Jeff Davis 2023-07-19 20:46:22 Re: Use of additional index columns in rows filtering