On Mon, Oct 11, 2021 at 02:38:12PM +0900, Michael Paquier wrote:
> On Fri, Oct 01, 2021 at 04:58:41PM +0900, Michael Paquier wrote:
> > I was looking at this CF entry, and what you are doing in 0004 to move
> > the tweaks from pg_upgrade's test.sh to a separate SQL script that
> > uses psql's meta-commands like \if to check which version we are on is
> > really interesting. The patch does not apply anymore, so this needs a
> > rebase. The entry has been switched as waiting on author by Tom, but
> > you did not update it after sending the new versions in [1]. I am
> > wondering if we could have something cleaner than just a set booleans
> > as you do here for each check, as that does not help with the
> > readability of the tests.
>
> And so, I am back at this thread, looking at the set of patches
> proposed from 0001 to 0004. The patches are rather messy and mix many
> things and concepts, but there are basically four things that stand
> out:
> - test.sh is completely broken when using PG >= 14 as new version
> because of the removal of the test tablespace. Older versions of
> pg_regress don't support --make-tablespacedir so I am fine to stick a
> couple of extra mkdirs for testtablespace/, expected/ and sql/ to
> allow the script to work properly for major upgrades as a workaround,
> but only if we use an old version. We need to do something here for
> HEAD and REL_14_STABLE.
> - The script would fail when using PG <= 11 as old version because of
> WITH OIDS relations. We need to do something down to REL_12_STABLE.
> I did not like much the approach taken to stick 4 ALTER TABLE queries
> though (the patch was actually failing here for me), so instead I have
> borrowed what the buildfarm has been doing with a DO block. That
> works fine, and that's more portable.
> - Not using --extra-float-digits with PG <= 11 as older version causes
> a bunch of diffs in the dumps, making the whole unreadable. The patch
> was doing that unconditionally for *all version*, which is not good.
> We should only do that on the versions that need it, and we know the
> old version number before taking any dumps so that's easy to check.
> - The addition of --wal-segsize and --allow-group-access breaks the
> script when using PG < 10 at initdb time as these got added in 11.
> With 10 getting EOL'd next year and per the lack of complaints, I am
> not excited to do anything here and I'd rather leave this out so as we
> keep coverage for those options across *all* major versions upgraded
> from 11~. The buildfarm has tests down to 9.2, but for devs my take
> is that this is enough for now.
Michael handled those in fa66b6d.
Note that the patch assumes that the "old version" being pg_upgraded has
commit 97f73a978: "Work around cross-version-upgrade issues created by commit
9e38c2bb5."
That may be good enough for test.sh, but if the kludges were moved to a .sql
script which was also run by the buildfarm (in stead of its hardcoded kludges),
then
it might be necessary to handle the additional stuff my patch did, like:
+ DROP TRANSFORM FOR integer LANGUAGE sql
CASCADE;"
+ DROP FUNCTION boxarea(box);"
+ DROP FUNCTION funny_dup17();"
+ DROP TABLE abstime_tbl;"
+ DROP TABLE reltime_tbl;"
+ DROP TABLE tinterval_tbl;"
+ DROP AGGREGATE
first_el_agg_any(anyelement);"
+ DROP AGGREGATE
array_cat_accum(anyarray);"
+ DROP OPERATOR @#@(NONE,bigint);"
Or, maybe it's guaranteed that the animals all run latest version of old
branches, in which case I think some of the BF's existing logic could be
dropped, which would help to reconcile these two scripts:
my $missing_funcs = q{drop function if exists
public.boxarea(box);
drop function if exists public.funny_dup17();
..
$prstmt = join(';',
'drop operator @#@ (NONE, bigint)',
..
'drop aggregate if exists
public.array_cat_accum(anyarray)',
> This is for the basics in terms of fixing test.sh and what should be
> backpatched. In this aspect patches 0001 and 0002 were a bit
> incorrect. I am not sure that 0003 is that interesting as designed as
> we would miss any new core types introduced.
We wouldn't miss new core types, because of the 2nd part of type_sanity which
tests that each core type was included in the "manytypes" table.
+-- And now a test on the previous test, checking that all core types are
+-- included in this table
+-- XXX or some other non-catalog table processed by pg_upgrade
+SELECT oid, typname, typtype, typelem, typarray, typarray FROM pg_type t
+WHERE typtype NOT IN ('p', 'c')
+-- reg* which cannot be pg_upgraded
+AND oid != ALL(ARRAY['regproc', 'regprocedure', 'regoper', 'regoperator',
'regconfig', 'regdictionary', 'regnamespace', 'regcollation']::regtype[])
+-- XML might be disabled at compile-time
+AND oid != ALL(ARRAY['xml', 'gtsvector', 'pg_node_tree', 'pg_ndistinct',
'pg_dependencies', 'pg_mcv_list', 'pg_brin_bloom_summary',
'pg_brin_minmax_multi_summary']::regtype[])
+AND NOT EXISTS (SELECT 1 FROM pg_type u WHERE u.typarray=t.oid) -- exclude
arrays
+AND NOT EXISTS (SELECT 1 FROM pg_attribute a WHERE a.atttypid=t.oid AND
a.attnum>0 AND a.attrelid='manytypes'::regclass);
> 0004 is something I'd like to get done on HEAD to ease the move of the
> pg_upgrade tests to TAP, but it could be made a bit easier to read by
> not having all those oldpgversion_XX_YY flags grouped together for
> one. So I am going to rewrite portions of it once done with the
> above.
--
Justin
>From 50261556825655c0f78459dd2a1cc310d88f55d6 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sat, 5 Dec 2020 17:20:09 -0600
Subject: [PATCH v6 1/2] pg_upgrade: test to exercise binary compatibility
Creating a table with columns of many different datatypes to notice if the
binary format is accidentally changed again, as happened at:
7c15cef86 Base information_schema.sql_identifier domain on name, not varchar.
I checked that if I cherry-pick to v11, and comment out
old_11_check_for_sql_identifier_data_type_usage(), then pg_upgrade/test.sh
detects the original problem:
pg_dump: error: Error message from server: ERROR: invalid memory alloc request size 18446744073709551613
I understand the buildfarm has its own cross-version-upgrade test, which I
think would catch this on its own.
---
src/test/regress/expected/sanity_check.out | 1 +
src/test/regress/expected/type_sanity.out | 55 ++++++++++++++++++++++
src/test/regress/sql/type_sanity.sql | 54 +++++++++++++++++++++
3 files changed, 110 insertions(+)
diff --git a/src/test/regress/expected/sanity_check.out b/src/test/regress/expected/sanity_check.out
index d04dc66db9..b4880ea3af 100644
--- a/src/test/regress/expected/sanity_check.out
+++ b/src/test/regress/expected/sanity_check.out
@@ -69,6 +69,7 @@ line_tbl|f
log_table|f
lseg_tbl|f
main_table|f
+manytypes|f
mlparted|f
mlparted1|f
mlparted11|f
diff --git a/src/test/regress/expected/type_sanity.out b/src/test/regress/expected/type_sanity.out
index f567fd378e..58013a8df3 100644
--- a/src/test/regress/expected/type_sanity.out
+++ b/src/test/regress/expected/type_sanity.out
@@ -674,3 +674,58 @@ WHERE p1.rngmultitypid IS NULL OR p1.rngmultitypid = 0;
----------+------------+---------------
(0 rows)
+-- Create a table with different data types, to exercise binary compatibility
+-- during pg_upgrade test
+CREATE TABLE manytypes AS SELECT
+'(11,12)'::point, '(1,1),(2,2)'::line,
+'((11,11),(12,12))'::lseg, '((11,11),(13,13))'::box,
+'((11,12),(13,13),(14,14))'::path AS openedpath, '[(11,12),(13,13),(14,14)]'::path AS closedpath,
+'((11,12),(13,13),(14,14))'::polygon, '1,1,1'::circle,
+'today'::date, 'now'::time, 'now'::timestamp, 'now'::timetz, 'now'::timestamptz, '12 seconds'::interval,
+'{"reason":"because"}'::json, '{"when":"now"}'::jsonb, '$.a[*] ? (@ > 2)'::jsonpath,
+'127.0.0.1'::inet, '127.0.0.0/8'::cidr, '00:01:03:86:1c:ba'::macaddr8, '00:01:03:86:1c:ba'::macaddr,
+2::int2, 4::int4, 8::int8, 4::float4, '8'::float8, pi()::numeric,
+'foo'::"char", 'c'::bpchar, 'abc'::varchar, 'name'::name, 'txt'::text, true::bool,
+E'\\xDEADBEEF'::bytea, B'10001'::bit, B'10001'::varbit AS varbit, '12.34'::money,
+'abc'::refcursor,
+'1 2'::int2vector, '1 2'::oidvector, format('%s=UC/%s', USER, USER)::aclitem,
+'a fat cat sat on a mat and ate a fat rat'::tsvector, 'fat & rat'::tsquery,
+'a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11'::uuid, '11'::xid8,
+'pg_class'::regclass, 'regtype'::regtype type, 'pg_monitor'::regrole,
+'pg_class'::regclass::oid, '(1,1)'::tid, '2'::xid, '3'::cid,
+'10:20:10,14,15'::txid_snapshot, '10:20:10,14,15'::pg_snapshot, '16/B374D848'::pg_lsn,
+1::information_schema.cardinal_number,
+'l'::information_schema.character_data,
+'n'::information_schema.sql_identifier,
+'now'::information_schema.time_stamp,
+'YES'::information_schema.yes_or_no,
+'venus'::planets, 'i16'::insenum,
+'(1,2)'::int4range, '{(1,2)}'::int4multirange,
+'(3,4)'::int8range, '{(3,4)}'::int8multirange,
+'(1,2)'::float8range, '{(1,2)}'::float8multirange,
+'(3,4)'::numrange, '{(3,4)}'::nummultirange,
+'(a,b)'::textrange, '{(a,b)}'::textmultirange,
+'(12.34, 56.78)'::cashrange, '{(12.34, 56.78)}'::cashmultirange,
+'(2020-01-02, 2021-02-03)'::daterange,
+'{(2020-01-02, 2021-02-03)}'::datemultirange,
+'(2020-01-02 03:04:05, 2021-02-03 06:07:08)'::tsrange,
+'{(2020-01-02 03:04:05, 2021-02-03 06:07:08)}'::tsmultirange,
+'(2020-01-02 03:04:05, 2021-02-03 06:07:08)'::tstzrange,
+'{(2020-01-02 03:04:05, 2021-02-03 06:07:08)}'::tstzmultirange,
+arrayrange(ARRAY[1,2], ARRAY[2,1]),
+arraymultirange(arrayrange(ARRAY[1,2], ARRAY[2,1]));
+-- And now a test on the previous test, checking that all core types are
+-- included in this table
+-- XXX or some other non-catalog table processed by pg_upgrade
+SELECT oid, typname, typtype, typelem, typarray, typarray FROM pg_type t
+WHERE typtype NOT IN ('p', 'c')
+-- reg* which cannot be pg_upgraded
+AND oid != ALL(ARRAY['regproc', 'regprocedure', 'regoper', 'regoperator', 'regconfig', 'regdictionary', 'regnamespace', 'regcollation']::regtype[])
+-- XML might be disabled at compile-time
+AND oid != ALL(ARRAY['xml', 'gtsvector', 'pg_node_tree', 'pg_ndistinct', 'pg_dependencies', 'pg_mcv_list', 'pg_brin_bloom_summary', 'pg_brin_minmax_multi_summary']::regtype[])
+AND NOT EXISTS (SELECT 1 FROM pg_type u WHERE u.typarray=t.oid) -- exclude arrays
+AND NOT EXISTS (SELECT 1 FROM pg_attribute a WHERE a.atttypid=t.oid AND a.attnum>0 AND a.attrelid='manytypes'::regclass);
+ oid | typname | typtype | typelem | typarray | typarray
+-----+---------+---------+---------+----------+----------
+(0 rows)
+
diff --git a/src/test/regress/sql/type_sanity.sql b/src/test/regress/sql/type_sanity.sql
index 404c3a2043..e98191f01f 100644
--- a/src/test/regress/sql/type_sanity.sql
+++ b/src/test/regress/sql/type_sanity.sql
@@ -495,3 +495,57 @@ WHERE pronargs != 2
SELECT p1.rngtypid, p1.rngsubtype, p1.rngmultitypid
FROM pg_range p1
WHERE p1.rngmultitypid IS NULL OR p1.rngmultitypid = 0;
+
+-- Create a table with different data types, to exercise binary compatibility
+-- during pg_upgrade test
+
+CREATE TABLE manytypes AS SELECT
+'(11,12)'::point, '(1,1),(2,2)'::line,
+'((11,11),(12,12))'::lseg, '((11,11),(13,13))'::box,
+'((11,12),(13,13),(14,14))'::path AS openedpath, '[(11,12),(13,13),(14,14)]'::path AS closedpath,
+'((11,12),(13,13),(14,14))'::polygon, '1,1,1'::circle,
+'today'::date, 'now'::time, 'now'::timestamp, 'now'::timetz, 'now'::timestamptz, '12 seconds'::interval,
+'{"reason":"because"}'::json, '{"when":"now"}'::jsonb, '$.a[*] ? (@ > 2)'::jsonpath,
+'127.0.0.1'::inet, '127.0.0.0/8'::cidr, '00:01:03:86:1c:ba'::macaddr8, '00:01:03:86:1c:ba'::macaddr,
+2::int2, 4::int4, 8::int8, 4::float4, '8'::float8, pi()::numeric,
+'foo'::"char", 'c'::bpchar, 'abc'::varchar, 'name'::name, 'txt'::text, true::bool,
+E'\\xDEADBEEF'::bytea, B'10001'::bit, B'10001'::varbit AS varbit, '12.34'::money,
+'abc'::refcursor,
+'1 2'::int2vector, '1 2'::oidvector, format('%s=UC/%s', USER, USER)::aclitem,
+'a fat cat sat on a mat and ate a fat rat'::tsvector, 'fat & rat'::tsquery,
+'a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11'::uuid, '11'::xid8,
+'pg_class'::regclass, 'regtype'::regtype type, 'pg_monitor'::regrole,
+'pg_class'::regclass::oid, '(1,1)'::tid, '2'::xid, '3'::cid,
+'10:20:10,14,15'::txid_snapshot, '10:20:10,14,15'::pg_snapshot, '16/B374D848'::pg_lsn,
+1::information_schema.cardinal_number,
+'l'::information_schema.character_data,
+'n'::information_schema.sql_identifier,
+'now'::information_schema.time_stamp,
+'YES'::information_schema.yes_or_no,
+'venus'::planets, 'i16'::insenum,
+'(1,2)'::int4range, '{(1,2)}'::int4multirange,
+'(3,4)'::int8range, '{(3,4)}'::int8multirange,
+'(1,2)'::float8range, '{(1,2)}'::float8multirange,
+'(3,4)'::numrange, '{(3,4)}'::nummultirange,
+'(a,b)'::textrange, '{(a,b)}'::textmultirange,
+'(12.34, 56.78)'::cashrange, '{(12.34, 56.78)}'::cashmultirange,
+'(2020-01-02, 2021-02-03)'::daterange,
+'{(2020-01-02, 2021-02-03)}'::datemultirange,
+'(2020-01-02 03:04:05, 2021-02-03 06:07:08)'::tsrange,
+'{(2020-01-02 03:04:05, 2021-02-03 06:07:08)}'::tsmultirange,
+'(2020-01-02 03:04:05, 2021-02-03 06:07:08)'::tstzrange,
+'{(2020-01-02 03:04:05, 2021-02-03 06:07:08)}'::tstzmultirange,
+arrayrange(ARRAY[1,2], ARRAY[2,1]),
+arraymultirange(arrayrange(ARRAY[1,2], ARRAY[2,1]));
+
+-- And now a test on the previous test, checking that all core types are
+-- included in this table
+-- XXX or some other non-catalog table processed by pg_upgrade
+SELECT oid, typname, typtype, typelem, typarray, typarray FROM pg_type t
+WHERE typtype NOT IN ('p', 'c')
+-- reg* which cannot be pg_upgraded
+AND oid != ALL(ARRAY['regproc', 'regprocedure', 'regoper', 'regoperator', 'regconfig', 'regdictionary', 'regnamespace', 'regcollation']::regtype[])
+-- XML might be disabled at compile-time
+AND oid != ALL(ARRAY['xml', 'gtsvector', 'pg_node_tree', 'pg_ndistinct', 'pg_dependencies', 'pg_mcv_list', 'pg_brin_bloom_summary', 'pg_brin_minmax_multi_summary']::regtype[])
+AND NOT EXISTS (SELECT 1 FROM pg_type u WHERE u.typarray=t.oid) -- exclude arrays
+AND NOT EXISTS (SELECT 1 FROM pg_attribute a WHERE a.atttypid=t.oid AND a.attnum>0 AND a.attrelid='manytypes'::regclass);
--
2.17.0
>From 5ab4b974464f9732d5c9362f3b92cd33653864b3 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sat, 6 Mar 2021 18:35:26 -0600
Subject: [PATCH v6 2/2] Move pg_upgrade kludges to sql script
NOTE, "IF EXISTS" isn't necessary in fa66b6dee
---
src/bin/pg_upgrade/test-upgrade.sql | 51 +++++++++++++++++++++++++++++
src/bin/pg_upgrade/test.sh | 48 +--------------------------
2 files changed, 52 insertions(+), 47 deletions(-)
create mode 100644 src/bin/pg_upgrade/test-upgrade.sql
diff --git a/src/bin/pg_upgrade/test-upgrade.sql b/src/bin/pg_upgrade/test-upgrade.sql
new file mode 100644
index 0000000000..74fad312cb
--- /dev/null
+++ b/src/bin/pg_upgrade/test-upgrade.sql
@@ -0,0 +1,51 @@
+-- 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).
+
+SELECT
+ ver <= 804 AS oldpgversion_le84,
+ ver < 1000 AS oldpgversion_lt10,
+ ver < 1200 AS oldpgversion_lt12,
+ ver < 1400 AS oldpgversion_lt14
+ FROM (SELECT current_setting('server_version_num')::int/100 AS ver) AS v;
+\gset
+
+\if :oldpgversion_le84
+DROP FUNCTION public.myfunc(integer);
+\endif
+
+\if :oldpgversion_lt10
+-- last in 9.6 -- commit 5ded4bd21
+DROP FUNCTION public.oldstyle_length(integer, text);
+\endif
+
+\if :oldpgversion_lt14
+-- last in v13 commit 7ca37fb04
+DROP FUNCTION IF EXISTS public.putenv(text);
+-- last in v13 commit 76f412ab3
+-- public.!=- This one is only needed for v11+ ??
+-- Note, until v10, operators could only be dropped one at a time
+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
+
+\if :oldpgversion_lt12
+-- WITH OIDS is not supported anymore in v12, so remove 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
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 8593488907..46a1ebb4ab 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -181,53 +181,7 @@ 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
+ psql -X -d regression -f "test-upgrade.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
--
2.17.0
