Re: pg_upgrade test for binary compatibility of core data types

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Cc: Jacob Champion <pchampion(at)vmware(dot)com>, tgl(at)sss(dot)pgh(dot)pa(dot)us, peter(dot)eisentraut(at)enterprisedb(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org, buschmann(at)nidsa(dot)net, andrew(at)dunslane(dot)net, noah(at)leadboat(dot)com, tomas(dot)vondra(at)2ndquadrant(dot)com, bruce(at)momjian(dot)us, andres(at)anarazel(dot)de
Subject: Re: pg_upgrade test for binary compatibility of core data types
Date: 2021-11-18 06:58:18
Message-ID: YZX5ijyrhvEgPAoE@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

On Wed, Nov 17, 2021 at 10:47:28PM -0600, Justin Pryzby wrote:
> I'm not sure if everything the buildfarm does is needed anymore, or if any of
> it could be removed now, rather than being implemented in test.sh.

+-- This file has a bunch of kludges needed for testing upgrades
across major versions
+-- It supports testing the most recent version of an old release (not
any arbitrary minor version).

This could be better-worded. Here is an idea:
--
-- SQL queries for major upgrade tests
--
-- This file includes a set of SQL queries to make a cluster to-be-upgraded
-- compatible with the version this file is on. This requires psql,
-- as per-version queries are controlled with a set of \if clauses.

+\if :oldpgversion_le84
+DROP FUNCTION public.myfunc(integer);
+\endif
We could retire this part for <= 8.4. The oldest version tested by
the buildfarm is 9.2.

+ psql -X -d regression -f "test-upgrade.sql" || psql_fix_sql_status=$?
Shouldn't we use an absolute path here? I was testing a VPATH build
and that was not working properly.

+-- commit 9e38c2bb5 and 97f73a978
+-- DROP AGGREGATE array_larger_accum(anyarray);
+DROP AGGREGATE array_cat_accum(anyarray);
+
+-- commit 76f412ab3
+-- DROP OPERATOR @#@(bigint,NONE);
+DROP OPERATOR @#@(NONE,bigint);
+\endif
The buildfarm does "CREATE OPERATOR @#@" and "CREATE AGGREGATE
array_larger_accum" when dealing with an old version between 9.5 and
13. Shouldn't we do the same and create those objects rather than a
plain DROP? What you are doing is not wrong, and it should allow
upgrades to work, but that's a bit inconsistent with the buildfarm in
terms of coverage.

+ ver >= 905 AND ver <= 1300 AS oldpgversion_95_13,
+ ver >= 906 AND ver <= 1300 AS oldpgversion_96_13,
+ ver >= 906 AND ver <= 1000 AS oldpgversion_96_10,
So here, we have the choice between conditions that play with version
ranges or we could make those checks simpler but compensate with a set
of IF EXISTS queries. I think that your choice is right. The
buildfarm mixes both styles to compensate with the cases where the
objects are created after a drop.

The list of objects and the version ranges look correct to me.
--
Michael

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Andres Freund 2021-11-18 07:19:41 Re: BUG #17255: Server crashes in index_delete_sort_cmp() due to race condition with vacuum
Previous Message Semab Tariq 2021-11-18 06:08:48 Re: BUG #17290: Error Installation

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2021-11-18 07:05:06 Re: XLogReadRecord() error in XlogReadTwoPhaseData()
Previous Message Michael Paquier 2021-11-18 05:49:35 Re: pg_upgrade test for binary compatibility of core data types