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