On Thu, Nov 18, 2021 at 03:58:18PM +0900, Michael Paquier wrote: > + 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.
So, I have come back to this part of the patch set, that moves the SQL queries doing the pre-upgrade cleanups in the old version we upgrade from, and decided to go with what looks like the simplest approach, relying on some IFEs depending on the object types if they don't exist for some cases. While checking the whole thing, I have noticed that some of the operations were not really necessary. The result is rather clean now, with a linear organization of the version logic, so as it is a no-brainer to get that done in back-branches per the backward-compatibility argument. I'll get that done down to 10 to maximize its influence, then I'll move on with the buildfarm code and send a patch to plug this and reduce the dependencies between core and the buildfarm code. -- Michael
From 9bfe54d8867c9c05a36976f01ed65e5b8da442f7 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Wed, 1 Dec 2021 16:08:01 +0900
Subject: [PATCH] Move SQLs for cleanups before cross-version upgrades into a
new file
The plan is to make the buildfarm code re-use this code, and test.sh
held a duplicated logic for this work. Separating those SQLs into a new
file with a set of \if clauses to do version checks with the old version
upgrading will allow the buildfarm to reuse that. An extra
simplification is that committers will be able to control the objects
cleaned up without any need to tweak the buldfarm code, at least for the
main regression test suite.
Backpatch down to 10, to maximize its effects.
---
src/bin/pg_upgrade/test.sh | 52 ++--------------
src/bin/pg_upgrade/upgrade_adapt.sql | 92 ++++++++++++++++++++++++++++
2 files changed, 97 insertions(+), 47 deletions(-)
create mode 100644 src/bin/pg_upgrade/upgrade_adapt.sql
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 8593488907..54c02bc65b 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -181,53 +181,11 @@ if "$MAKE" -C "$oldsrc" installcheck-parallel; then
# Before dumping, tweak the database of the old instance depending
# on its version.
if [ "$newsrc" != "$oldsrc" ]; then
- fix_sql=""
- # Get rid of objects not feasible in later versions
- case $oldpgversion in
- 804??)
- fix_sql="DROP FUNCTION public.myfunc(integer);"
- ;;
- esac
-
- # Last appeared in v9.6
- if [ $oldpgversion -lt 100000 ]; then
- fix_sql="$fix_sql
- DROP FUNCTION IF EXISTS
- public.oldstyle_length(integer, text);"
- fi
- # Last appeared in v13
- if [ $oldpgversion -lt 140000 ]; then
- fix_sql="$fix_sql
- DROP FUNCTION IF EXISTS
- public.putenv(text); -- last in v13
- DROP OPERATOR IF EXISTS -- last in v13
- public.#@# (pg_catalog.int8, NONE),
- public.#%# (pg_catalog.int8, NONE),
- public.!=- (pg_catalog.int8, NONE),
- public.#@%# (pg_catalog.int8, NONE);"
- fi
- psql -X -d regression -c "$fix_sql;" || psql_fix_sql_status=$?
-
- # WITH OIDS is not supported anymore in v12, so remove support
- # for any relations marked as such.
- if [ $oldpgversion -lt 120000 ]; then
- fix_sql="DO \$stmt\$
- DECLARE
- rec text;
- BEGIN
- FOR rec in
- SELECT oid::regclass::text
- FROM pg_class
- WHERE relname !~ '^pg_'
- AND relhasoids
- AND relkind in ('r','m')
- ORDER BY 1
- LOOP
- execute 'ALTER TABLE ' || rec || ' SET WITHOUT OIDS';
- END LOOP;
- END; \$stmt\$;"
- psql -X -d regression -c "$fix_sql;" || psql_fix_sql_status=$?
- fi
+ # This SQL script has its own idea of the cleanup that needs to be
+ # done and embeds version checks. Note that this uses the script
+ # stored on the new branch.
+ psql -X -d regression -f "$newsrc/src/bin/pg_upgrade/upgrade_adapt.sql" \
+ || psql_fix_sql_status=$?
# Handling of --extra-float-digits gets messy after v12.
# Note that this changes the dumps from the old and new
diff --git a/src/bin/pg_upgrade/upgrade_adapt.sql b/src/bin/pg_upgrade/upgrade_adapt.sql
new file mode 100644
index 0000000000..175b2ebe2e
--- /dev/null
+++ b/src/bin/pg_upgrade/upgrade_adapt.sql
@@ -0,0 +1,92 @@
+--
+-- SQL queries for upgrade tests across different major versions.
+--
+-- This file includes a set of SQL queries to make a cluster to-be-upgraded
+-- compatible with the version this file is based on. Note that this
+-- requires psql, as per-version queries are controlled with a set of \if
+-- clauses.
+
+-- This script is backward-compatible, so it is able to work with any version
+-- newer than 9.2 we are upgrading from, up to the branch this script is stored
+-- on (even if this would not run if running pg_upgrade with the same version
+-- for the origin and the target).
+
+-- \if accepts a simple boolean value, so all the version checks are
+-- done based on this assumption.
+SELECT
+ ver <= 902 AS oldpgversion_le92,
+ ver <= 904 AS oldpgversion_le94,
+ ver <= 906 AS oldpgversion_le96,
+ ver <= 1000 AS oldpgversion_le10,
+ ver <= 1100 AS oldpgversion_le11,
+ ver <= 1300 AS oldpgversion_le13
+ FROM (SELECT current_setting('server_version_num')::int / 100 AS ver) AS v;
+\gset
+
+-- Objects last appearing in 9.2.
+\if :oldpgversion_le92
+-- Note that those tables are removed from the regression tests in 9.3
+-- and newer versions.
+DROP TABLE abstime_tbl;
+DROP TABLE reltime_tbl;
+DROP TABLE tinterval_tbl;
+\endif
+
+-- Objects last appearing in 9.4.
+\if :oldpgversion_le94
+-- This aggregate has been fixed in 9.5 and later versions, so drop
+-- and re-create it.
+DROP AGGREGATE array_cat_accum(anyarray);
+CREATE AGGREGATE array_larger_accum (anyarray) (
+ sfunc = array_larger,
+ stype = anyarray,
+ initcond = $${}$$);
+-- This operator has been fixed in 9.5 and later versions, so drop and
+-- re-create it.
+DROP OPERATOR @#@ (NONE, bigint);
+CREATE OPERATOR @#@ (PROCEDURE = factorial,
+ RIGHTARG = bigint);
+\endif
+
+-- Objects last appearing in 9.6.
+\if :oldpgversion_le96
+DROP FUNCTION public.oldstyle_length(integer, text);
+\endif
+
+-- Objects last appearing in 10.
+\if :oldpgversion_le10
+DROP FUNCTION IF EXISTS boxarea(box);
+DROP FUNCTION IF EXISTS funny_dup17();
+\endif
+
+-- Objects last appearing in 11.
+\if :oldpgversion_le11
+-- WITH OIDS is supported until v11, so remove its support for any
+-- relations marked as such.
+DO $stmt$
+ DECLARE
+ rec text;
+ BEGIN
+ FOR rec in
+ SELECT oid::regclass::text
+ FROM pg_class
+ WHERE relname !~ '^pg_'
+ AND relhasoids
+ AND relkind in ('r','m')
+ ORDER BY 1
+ LOOP
+ execute 'ALTER TABLE ' || rec || ' SET WITHOUT OIDS';
+ END LOOP;
+ END; $stmt$;
+\endif
+
+-- Objects last appearing in 13.
+\if :oldpgversion_le13
+DROP FUNCTION IF EXISTS public.putenv(text);
+-- Until v10, operators could only be dropped one at a time, so be careful
+-- to stick with one command for each drop here.
+DROP OPERATOR public.#@# (pg_catalog.int8, NONE);
+DROP OPERATOR public.#%# (pg_catalog.int8, NONE);
+DROP OPERATOR public.!=- (pg_catalog.int8, NONE);
+DROP OPERATOR public.#@%# (pg_catalog.int8, NONE);
+\endif
--
2.34.0
signature.asc
Description: PGP signature
