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 19:20:22
Message-ID: bf5eee7b-fed5-7a1c-110b-a58e820bd104@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On 2023-07-19 We 12:05, Alvaro Herrera wrote:
> On 2023-Jul-19, Andrew Dunstan wrote:
>
>> On 2023-07-19 We 07:05, Alvaro Herrera wrote:
>>> I just hit a snag testing this. It turns out that the
>>> PostgreSQL::Version comparison stuff believes that 16beta2 < 16, which
>>> sounds reasonable. However, because of that, the AdjustUpgrade.pm
>>> stanza that tries to drop tables public.gtest_normal_child{2} in
>>> versions earlier than 16 fails, because by 16 these tables are dropped
>>> in the test itself rather than left to linger, as was the case in
>>> versions 15 and earlier.
>> The buildfarm module assumes that no adjustments are necessary if the old
>> and new versions are the same (e.g. HEAD to HEAD). And it never passes in a
>> version like '16beta2'. It extracts the version number from the branch name,
>> e.g. REL_16_STABLE => 16.
> Hmm, OK, but I'm not testing the same versions -- I'm testing 16beta2 to
> 17devel.

Yeah, but you asked why the buildfarm didn't see this effect, and the
answer is that it never uses version arguments like '16beta2'.

>
>>> I can fix this either by using DROP IF EXISTS in that stanza, or by
>>> making AdjustUpgrade use 'version <= 15'. Any opinions on which to
>>> prefer?
>> The trouble is this could well break the next time someone puts in a test
>> like this.
> Hmm, I don't understand what you mean.

I want to prevent things like this from happening in the future if
someone puts a test in the development branch with  "if ($oldversion < nn)".

>
>> 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.

cheers

andrew

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2023-07-19 19:38:12 Re: Adding argument names to aggregate functions
Previous Message John Morris 2023-07-19 18:55:10 Re: Atomic ops for unlogged LSN