Re: Extracting cross-version-upgrade knowledge from buildfarm client
Andrew Dunstan <andrew@dunslane.net>
From: Andrew Dunstan <andrew@dunslane.net>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers@lists.postgresql.org
Date: 2023-07-19T19:20:22Z
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
Commits
-
Compare only major versions in AdjustUpgrade.pm
- 6061adedf53c 17.0 landed
- 28ce9d51f908 16.0 landed
-
Get rid of the "new" and "old" entries in a view's rangetable.
- 47bb9db75996 16.0 landed
-
Create common infrastructure for cross-version upgrade testing.
- 52585f8f072a 16.0 landed
- f02a75222821 11.19 landed
- ddd89df26b97 10 (unreleased) landed
- bfa3d2790664 9.5 (unreleased) landed
- a2091843f69e 9.2 (unreleased) landed
- 8e7398dce55b 14.7 landed
- 4bcf3521290a 9.4 (unreleased) landed
- 4ad0896bca92 15.2 landed
- 3f0b9df88bb3 13.10 landed
- 3911fd55f53a 9.6 (unreleased) landed
- 2e8bedc3d68c 9.3 (unreleased) landed
- 0e4d38100acb 12.14 landed