Re: Extracting cross-version-upgrade knowledge from buildfarm client - Mailing list pgsql-hackers

From Andrew Dunstan
Subject Re: Extracting cross-version-upgrade knowledge from buildfarm client
Date
Msg-id bf5eee7b-fed5-7a1c-110b-a58e820bd104@dunslane.net
Whole thread Raw
In response to Re: Extracting cross-version-upgrade knowledge from buildfarm client  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
Responses Re: Extracting cross-version-upgrade knowledge from buildfarm client
List 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

pgsql-hackers by date:

Previous
From: John Morris
Date:
Subject: Re: Atomic ops for unlogged LSN
Next
From: Daniel Gustafsson
Date:
Subject: Re: Adding argument names to aggregate functions