Date: Sun, 6 Dec 2020 12:02:48 -0600
From: Justin Pryzby <pryzby@telsasoft.com>
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Andres Freund <andres@anarazel.de>,
	buschmann@nidsa.net, pgsql-hackers@lists.postgresql.org
Subject: pg_upgrade test for binary compatibility of core data types
Message-ID: <20201206180248.GI24052@telsasoft.com>
References: <1542.1570738460@sss.pgh.pa.us>
 <20191010204022.6hiiubm2cueu3ghk@development>
 <20191013001032.lqrn4kqaovhrf77l@development>
 <17717.1570991208@sss.pgh.pa.us>
 <20191013203848.2eksimmpux3546wr@development>
 <27596.1571062600@sss.pgh.pa.us>
 <20191014163538.cqn6j4cl7s2mk2yz@development>
 <20191015001817.zei45apgpu5dzngr@development>
 <20191015044118.GO3599@telsasoft.com>
 <20191015070725.csmdmdvrlb6klwkg@development>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="WQS/f3aCdEH+xDC1"
Content-Disposition: inline
In-Reply-To: <20191015070725.csmdmdvrlb6klwkg@development>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk


--WQS/f3aCdEH+xDC1
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

I'm finally returning to this 14 month old thread:
(was: Re: BUG #16045: vacuum_db crash and illegal memory alloc after pg_upgrade from PG11 to PG12)

On Tue, Oct 15, 2019 at 09:07:25AM +0200, Tomas Vondra wrote:
> On Mon, Oct 14, 2019 at 11:41:18PM -0500, Justin Pryzby wrote:
> > 
> > Perhaps it'd be worth creating a test for on-disk format ?
> > 
> > Like a table with a column for each core type, which is either SELECTed from
> > after pg_upgrade, or pg_dump output compared before and after.
> 
> IMO that would be useful - we now have a couple of these checks for
> different data types (line, unknown, sql_identifier), with a couple of
> combinations each. And I've been looking if we do similar pg_upgrade
> tests, but I haven't found anything. I mean, we do pg_upgrade the
> cluster used for regression tests, but here we need to test a number of
> cases that are meant to abort the pg_upgrade. So we'd need a number of
> pg_upgrade runs, to test that.

I meant to notice if the binary format is accidentally changed again, which was
what happened here:
7c15cef86 Base information_schema.sql_identifier domain on name, not varchar.

I added a table to the regression tests so it's processed by pg_upgrade tests,
run like:

| time make -C src/bin/pg_upgrade check oldsrc=`pwd`/11 oldbindir=`pwd`/11/tmp_install/usr/local/pgsql/bin

I checked that if I cherry-pick 0002 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.

These all seem to complicate use of pg_upgrade/test.sh, so 0001 is needed to
allow testing upgrade from older releases.

e78900afd217fa3eaa77c51e23a94c1466af421c Create by default sql/ and expected/ for output directory in pg_regress
40b132c1afbb4b1494aa8e48cc35ec98d2b90777 In the pg_upgrade test suite, don't write to src/test/regress.
fc49e24fa69a15efacd5b8958115ed9c43c48f9a Make WAL segment size configurable at initdb time.
c37b3d08ca6873f9d4eaf24c72a90a550970cbb8 Allow group access on PGDATA
da9b580d89903fee871cf54845ffa2b26bda2e11 Refactor dir/file permissions

-- 
Justin

--WQS/f3aCdEH+xDC1
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0001-WIP-pg_upgrade-test.sh-changes-needed-to-allow-testi.patch"

From e9d70f3d043211011f7c7774ed2ed5eaee3760dc Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 5 Dec 2020 22:31:19 -0600
Subject: [PATCH 1/2] WIP: pg_upgrade/test.sh: changes needed to allow testing
 upgrade from v11

---
 src/bin/pg_upgrade/check.c |  2 +-
 src/bin/pg_upgrade/test.sh | 44 ++++++++++++++++++++++++++++++++------
 2 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 357997972b..6dfe3cff65 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -122,7 +122,7 @@ check_and_dump_old_cluster(bool live_check)
 	 * to prevent upgrade when used in user objects (tables, indexes, ...).
 	 */
 	if (GET_MAJOR_VERSION(old_cluster.major_version) <= 1100)
-		old_11_check_for_sql_identifier_data_type_usage(&old_cluster);
+		; // old_11_check_for_sql_identifier_data_type_usage(&old_cluster);
 
 	/*
 	 * Pre-PG 10 allowed tables with 'unknown' type columns and non WAL logged
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 04aa7fd9f5..b39265f66d 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -23,7 +23,7 @@ standard_initdb() {
 	# To increase coverage of non-standard segment size and group access
 	# without increasing test runtime, run these tests with a custom setting.
 	# Also, specify "-A trust" explicitly to suppress initdb's warning.
-	"$1" -N --wal-segsize 1 -g -A trust
+	"$1" -N -A trust
 	if [ -n "$TEMP_CONFIG" -a -r "$TEMP_CONFIG" ]
 	then
 		cat "$TEMP_CONFIG" >> "$PGDATA/postgresql.conf"
@@ -108,6 +108,9 @@ export EXTRA_REGRESS_OPTS
 mkdir "$outputdir"
 mkdir "$outputdir"/testtablespace
 
+mkdir "$outputdir"/sql
+mkdir "$outputdir"/expected
+
 logdir=`pwd`/log
 rm -rf "$logdir"
 mkdir "$logdir"
@@ -175,13 +178,36 @@ if "$MAKE" -C "$oldsrc" installcheck-parallel; then
 				fix_sql="DROP FUNCTION public.myfunc(integer); DROP FUNCTION public.oldstyle_length(integer, text);"
 				;;
 			*)
-				fix_sql="DROP FUNCTION public.oldstyle_length(integer, text);"
+				fix_sql="DROP FUNCTION IF EXISTS public.oldstyle_length(integer, text);"
+
+				# commit 1ed6b8956
+				fix_sql="$fix_sql DROP OPERATOR public.#@# (pg_catalog.int8, NONE);"
+				fix_sql="$fix_sql DROP OPERATOR public.#%# (pg_catalog.int8, NONE);"
+				fix_sql="$fix_sql DROP OPERATOR public.!=- (pg_catalog.int8, NONE);"
+				fix_sql="$fix_sql DROP OPERATOR public.#@%# (pg_catalog.int8, NONE);"
+
+				# commit 76f412ab3
+				fix_sql="$fix_sql DROP OPERATOR IF EXISTS @#@(bigint,NONE);"
+				fix_sql="$fix_sql DROP OPERATOR IF EXISTS @#@(NONE,bigint);"
+
+				# commit 9e38c2bb5 and 97f73a978
+				fix_sql="$fix_sql DROP AGGREGATE IF EXISTS array_larger_accum (anyarray);"
+				fix_sql="$fix_sql DROP AGGREGATE IF EXISTS array_cat_accum(anyarray);"
+				fix_sql="$fix_sql DROP AGGREGATE IF EXISTS first_el_agg_any(anyelement);"
+
+				# commit 578b22971
+				fix_sql="$fix_sql ALTER TABLE public.tenk1 SET WITHOUT OIDS;"
+				fix_sql="$fix_sql ALTER TABLE public.tenk1 SET WITHOUT OIDS;"
+				#fix_sql="$fix_sql ALTER TABLE public.stud_emp SET WITHOUT OIDS;" # inherited
+				fix_sql="$fix_sql ALTER TABLE public.emp SET WITHOUT OIDS;"
+				fix_sql="$fix_sql ALTER TABLE public.tt7 SET WITHOUT OIDS;"
 				;;
 		esac
+
 		psql -X -d regression -c "$fix_sql;" || psql_fix_sql_status=$?
 	fi
 
-	pg_dumpall --no-sync -f "$temp_root"/dump1.sql || pg_dumpall1_status=$?
+	pg_dumpall --extra-float-digits=0 --no-sync -f "$temp_root"/dump1.sql || pg_dumpall1_status=$?
 
 	if [ "$newsrc" != "$oldsrc" ]; then
 		# update references to old source tree's regress.so etc
@@ -227,23 +253,29 @@ pg_upgrade $PG_UPGRADE_OPTS -d "${PGDATA}.old" -D "$PGDATA" -b "$oldbindir" -p "
 # Windows hosts don't support Unix-y permissions.
 case $testhost in
 	MINGW*) ;;
-	*)	if [ `find "$PGDATA" -type f ! -perm 640 | wc -l` -ne 0 ]; then
+	*)
+		x=`find "$PGDATA" -type f -perm /127 -ls`
+		if [ -n "$x" ]; then
 			echo "files in PGDATA with permission != 640";
+			echo "$x" |head
 			exit 1;
 		fi ;;
 esac
 
 case $testhost in
 	MINGW*) ;;
-	*)	if [ `find "$PGDATA" -type d ! -perm 750 | wc -l` -ne 0 ]; then
+	*)
+		x=`find "$PGDATA" -type d -perm 027 -ls`
+		if [ "$x" ]; then
 			echo "directories in PGDATA with permission != 750";
+			echo "$x" |head
 			exit 1;
 		fi ;;
 esac
 
 pg_ctl start -l "$logdir/postmaster2.log" -o "$POSTMASTER_OPTS" -w
 
-pg_dumpall --no-sync -f "$temp_root"/dump2.sql || pg_dumpall2_status=$?
+pg_dumpall --extra-float-digits=0 --no-sync -f "$temp_root"/dump2.sql || pg_dumpall2_status=$?
 pg_ctl -m fast stop
 
 if [ -n "$pg_dumpall2_status" ]; then
-- 
2.17.0


--WQS/f3aCdEH+xDC1
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0002-pg_upgrade-test-to-exercise-binary-compatibility.patch"

From 7cdcf46561948a2011a88945bac7d05cb1f13baa Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 5 Dec 2020 17:20:09 -0600
Subject: [PATCH 2/2] pg_upgrade: test to exercise binary compatibility

Creating a table with columns of many different datatypes.
---
 src/test/regress/expected/sanity_check.out |  1 +
 src/test/regress/expected/type_sanity.out  | 42 ++++++++++++++++++++++
 src/test/regress/sql/type_sanity.sql       | 42 ++++++++++++++++++++++
 3 files changed, 85 insertions(+)

diff --git a/src/test/regress/expected/sanity_check.out b/src/test/regress/expected/sanity_check.out
index 192445878d..aa0a4fd9be 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 274130e706..2feefc7224 100644
--- a/src/test/regress/expected/type_sanity.out
+++ b/src/test/regress/expected/type_sanity.out
@@ -631,3 +631,45 @@ WHERE pronargs != 2
 ----------+------------+---------
 (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,
+'<foo>bar</foo>'::xml,
+'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,
+'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, 'english'::regconfig, 'simple'::regdictionary, 'pg_catalog'::regnamespace, 
+-- -- 'POSIX'::regcollation,
+-- -- '+'::regoper,
+-- '*(integer,integer)'::regoperator,
+-- -- 'sum'::regproc,
+-- 'sum(int4)'::regprocedure, USER::regrole, 'regtype'::regtype type,
+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;
+-- And now a test on the previous test, checking that all core types are
+-- included in this table (or some other non-catalog table processed by pg_upgrade).
+SELECT typname, typtype, typelem, typarray, typarray FROM pg_type t
+WHERE typnamespace IN ('pg_catalog'::regnamespace, 'information_schema'::regnamespace)
+AND typtype IN ('b', 'e', 'd')
+AND NOT typname~'_|^char$|^reg'
+AND oid != ALL(ARRAY['gtsvector', 'regcollation', 'regoper', 'regproc']::regtype[])
+AND NOT EXISTS (SELECT * FROM pg_attribute a WHERE a.atttypid=t.oid AND a.attrelid='manytypes'::regclass)
+ORDER BY 1,2,3,4;
+ 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 4b492ce062..f2b490a9c6 100644
--- a/src/test/regress/sql/type_sanity.sql
+++ b/src/test/regress/sql/type_sanity.sql
@@ -467,3 +467,45 @@ FROM pg_range p1 JOIN pg_proc p ON p.oid = p1.rngsubdiff
 WHERE pronargs != 2
     OR proargtypes[0] != rngsubtype OR proargtypes[1] != rngsubtype
     OR prorettype != 'pg_catalog.float8'::regtype;
+
+-- 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,
+'<foo>bar</foo>'::xml,
+'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,
+'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, 'english'::regconfig, 'simple'::regdictionary, 'pg_catalog'::regnamespace, 
+-- -- 'POSIX'::regcollation,
+-- -- '+'::regoper,
+-- '*(integer,integer)'::regoperator,
+-- -- 'sum'::regproc,
+-- 'sum(int4)'::regprocedure, USER::regrole, 'regtype'::regtype type,
+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;
+
+-- And now a test on the previous test, checking that all core types are
+-- included in this table (or some other non-catalog table processed by pg_upgrade).
+SELECT typname, typtype, typelem, typarray, typarray FROM pg_type t
+WHERE typnamespace IN ('pg_catalog'::regnamespace, 'information_schema'::regnamespace)
+AND typtype IN ('b', 'e', 'd')
+AND NOT typname~'_|^char$|^reg'
+AND oid != ALL(ARRAY['gtsvector', 'regcollation', 'regoper', 'regproc']::regtype[])
+AND NOT EXISTS (SELECT * FROM pg_attribute a WHERE a.atttypid=t.oid AND a.attrelid='manytypes'::regclass)
+ORDER BY 1,2,3,4;
-- 
2.17.0


--WQS/f3aCdEH+xDC1--