On Sun, Nov 07, 2021 at 01:22:00PM -0600, Justin Pryzby wrote: > 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();"
These apply for an old version <= v10. > + DROP TABLE abstime_tbl;" > + DROP TABLE reltime_tbl;" > + DROP TABLE tinterval_tbl;" old version <= 9.3. > + DROP AGGREGATE > first_el_agg_any(anyelement);" Not sure about this one. > + DROP AGGREGATE > array_cat_accum(anyarray);" > + DROP OPERATOR @#@(NONE,bigint);" These are on 9.4. It is worth noting that TestUpgradeXversion.pm recreates those objects. I'd agree to close the gap completely rather than just moving what test.sh does to wipe out a maximum client code for the buildfarm. > 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: That seems like a worthy goal to reduce the amount of duplication with the buildfarm code, while allowing tests from upgrades with older versions (the WAL segment size and group permission issue in test.sh had better be addressed in a better way, perhaps once the pg_upgrade tests are moved to TAP). There are also things specific to contrib/ modules with older versions, but that may be too specific for this exercise. +\if :oldpgversion_le84 +DROP FUNCTION public.myfunc(integer); +\endif The oldest version tested by the buildfarm is 9.2, so we could ignore this part I guess? Andrew, what do you think about this part? Based on my read of this thread, there is an agreement that this approach makes the buildfarm code more manageable so as committers would not need to patch the buildfarm code if their test fail. I agree with this conclusion, but I wanted to double-check with you first. This would need a backpatch down to 10 so as we could clean up a maximum of code in TestUpgradeXversion.pm without waiting for an extra 5 years. Please note that I am fine to send a patch for the buildfarm client. > 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. Thanks, I see your point now after a closer read. There is still a pending question for contrib modules, but I think that we need to think larger here with a better integration of contrib/ modules in the upgrade testing process. Making that cheap would require running the set of regression tests on the instance to-be-upgraded first. I think that one step in this direction would be to have unique databases for each contrib/ modules, so as there is no overlap with objects dropped? Having some checks with code types looks fine as a first step, so let's do that. I have reviewed 0001, rewrote a couple of comments. All the comments from upthread seem to be covered with that. So I'd like to get that applied on HEAD. We could as well be less conservative and backpatch that down to 12 to follow on 7c15cef so we would be more careful with 15~ already (a backpatch down to 14 would be enough for this purpose, actually thanks to the 14->15 upgrade path). -- Michael
From c4d766f9a461dad2d51cba3cb8d7d0c523267716 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Wed, 17 Nov 2021 15:57:04 +0900
Subject: [PATCH v7] 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 | 102 +++++++++++++++++++++
src/test/regress/sql/type_sanity.sql | 100 ++++++++++++++++++++
3 files changed, 203 insertions(+)
diff --git a/src/test/regress/expected/sanity_check.out b/src/test/regress/expected/sanity_check.out
index d04dc66db9..63706a28cc 100644
--- a/src/test/regress/expected/sanity_check.out
+++ b/src/test/regress/expected/sanity_check.out
@@ -185,6 +185,7 @@ sql_parts|f
sql_sizing|f
stud_emp|f
student|f
+tab_core_types|f
tableam_parted_a_heap2|f
tableam_parted_b_heap2|f
tableam_parted_c_heap2|f
diff --git a/src/test/regress/expected/type_sanity.out b/src/test/regress/expected/type_sanity.out
index f567fd378e..3ffd9d0d71 100644
--- a/src/test/regress/expected/type_sanity.out
+++ b/src/test/regress/expected/type_sanity.out
@@ -674,3 +674,105 @@ WHERE p1.rngmultitypid IS NULL OR p1.rngmultitypid = 0;
----------+------------+---------------
(0 rows)
+-- Create a table that holds all the known in-core data types and leave it
+-- around so as pg_upgrade is able to test their binary compatibility.
+CREATE TABLE tab_core_types 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]));
+-- Sanity check on the previous table, checking that all core types are
+-- included in this table.
+SELECT oid, typname, typtype, typelem, typarray, typarray
+ FROM pg_type t
+ WHERE typtype NOT IN ('p', 'c') AND
+ -- reg* types cannot be pg_upgraded, so discard them.
+ oid != ALL(ARRAY['regproc', 'regprocedure', 'regoper',
+ 'regoperator', 'regconfig', 'regdictionary',
+ 'regnamespace', 'regcollation']::regtype[]) AND
+ -- Discard types that do not accept input values as these cannot be
+ -- tested easily.
+ -- Note: XML might be disabled at compile-time.
+ oid != ALL(ARRAY['gtsvector', 'pg_node_tree',
+ 'pg_ndistinct', 'pg_dependencies', 'pg_mcv_list',
+ 'pg_brin_bloom_summary',
+ 'pg_brin_minmax_multi_summary', 'xml']::regtype[]) AND
+ -- Discard arrays.
+ NOT EXISTS (SELECT 1 FROM pg_type u WHERE u.typarray = t.oid)
+ -- Exclude everything from the table created above. This checks
+ -- that no in-core types are missing in tab_core_types.
+ AND NOT EXISTS (SELECT 1
+ FROM pg_attribute a
+ WHERE a.atttypid=t.oid AND
+ a.attnum > 0 AND
+ a.attrelid='tab_core_types'::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..f92773b75e 100644
--- a/src/test/regress/sql/type_sanity.sql
+++ b/src/test/regress/sql/type_sanity.sql
@@ -495,3 +495,103 @@ 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 that holds all the known in-core data types and leave it
+-- around so as pg_upgrade is able to test their binary compatibility.
+CREATE TABLE tab_core_types 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]));
+
+-- Sanity check on the previous table, checking that all core types are
+-- included in this table.
+SELECT oid, typname, typtype, typelem, typarray, typarray
+ FROM pg_type t
+ WHERE typtype NOT IN ('p', 'c') AND
+ -- reg* types cannot be pg_upgraded, so discard them.
+ oid != ALL(ARRAY['regproc', 'regprocedure', 'regoper',
+ 'regoperator', 'regconfig', 'regdictionary',
+ 'regnamespace', 'regcollation']::regtype[]) AND
+ -- Discard types that do not accept input values as these cannot be
+ -- tested easily.
+ -- Note: XML might be disabled at compile-time.
+ oid != ALL(ARRAY['gtsvector', 'pg_node_tree',
+ 'pg_ndistinct', 'pg_dependencies', 'pg_mcv_list',
+ 'pg_brin_bloom_summary',
+ 'pg_brin_minmax_multi_summary', 'xml']::regtype[]) AND
+ -- Discard arrays.
+ NOT EXISTS (SELECT 1 FROM pg_type u WHERE u.typarray = t.oid)
+ -- Exclude everything from the table created above. This checks
+ -- that no in-core types are missing in tab_core_types.
+ AND NOT EXISTS (SELECT 1
+ FROM pg_attribute a
+ WHERE a.atttypid=t.oid AND
+ a.attnum > 0 AND
+ a.attrelid='tab_core_types'::regclass);
--
2.33.1
signature.asc
Description: PGP signature
