From: Daniel Gustafsson <daniel@yesql.se>
Message-Id: <E0EB10CE-9B58-44C0-8CBC-807BC8E5A84A@yesql.se>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_F9A21101-F27B-47B9-8C10-08E797BA3490"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.3\))
Subject: Re: pgsql: Clean up role created in new subscription test.
Date: Fri, 1 Dec 2023 12:22:50 +0100
In-Reply-To: <202311081232.rn3aho5ay6yx@alvherre.pgsql>
Cc: Peter Eisentraut <peter@eisentraut.org>,
 Tom Lane <tgl@sss.pgh.pa.us>,
 Robert Haas <robertmhaas@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
References: <202311081232.rn3aho5ay6yx@alvherre.pgsql>
Archived-At: 
 <https://www.postgresql.org/message-id/E0EB10CE-9B58-44C0-8CBC-807BC8E5A84A%40yesql.se>
Precedence: bulk


--Apple-Mail=_F9A21101-F27B-47B9-8C10-08E797BA3490
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii

> On 8 Nov 2023, at 13:32, Alvaro Herrera <alvherre@alvh.no-ip.org> =
wrote:

> I suppose you're just thinking of using PQexec() or whatever, run one
> query with sufficient ORDER BY, save the result, and at the end of the
> test run just run the same query and compare that they are =
cell-by-cell
> identical?  This sounds a lot simpler than the patch you posted.

I found some spare cycles for this and came up with the attached.  The =
idea was
to keep it in-line with how pg_regress already today manipulate and =
traverse
_stringlists for various things.  With the addition of the 0001 patch to =
clean
up global objects left in test_pg_dump it passes check-world.

--
Daniel Gustafsson


--Apple-Mail=_F9A21101-F27B-47B9-8C10-08E797BA3490
Content-Disposition: attachment;
	filename=v4-0002-pg_regress-Detect-global-objects-left-over-after-.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v4-0002-pg_regress-Detect-global-objects-left-over-after-.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=201862409f720019249f22b059eaecc8fa11294c74=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20Daniel=20Gustafsson=20=
<dgustafsson@postgresql.org>=0ADate:=20Fri,=201=20Dec=202023=2012:10:17=20=
+0100=0ASubject:=20[PATCH=20v4=202/2]=20pg_regress:=20Detect=20global=20=
objects=20left=20over=20after=20test=0A=0AThe=20project=20policy=20is=20=
that=20regress=20tests=20should=20not=20leave=20any=0Aglobal=20objects=20=
behind,=20but=20there=20was=20no=20check=20enforcing=20this.=0A=
pg_regress=20will=20now=20extract=20a=20list=20of=20roles,=20tablespaces=20=
and=0Asubscriptions=20before=20and=20after=20running=20the=20test(s),=20=
and=20then=0Acompares=20the=20lists=20to=20ensure=20no=20new=20objects=20=
are=20present.=0A=0ADiscussion:=20=
https://postgr.es/m/CA+TgmoZ+K2OU0Ojs8_4SgqPWJJq7e0XiLd-uJGZ11R_WKF1URQ@ma=
il.gmail.com=0A---=0A=20src/test/regress/pg_regress.c=20|=2089=20=
+++++++++++++++++++++++++++++++++++=0A=201=20file=20changed,=2089=20=
insertions(+)=0A=0Adiff=20--git=20a/src/test/regress/pg_regress.c=20=
b/src/test/regress/pg_regress.c=0Aindex=20a9b8246cb7..c30a853c8c=20=
100644=0A---=20a/src/test/regress/pg_regress.c=0A+++=20=
b/src/test/regress/pg_regress.c=0A@@=20-132,6=20+132,8=20@@=20static=20=
char=20sockself[MAXPGPATH];=0A=20static=20char=20socklock[MAXPGPATH];=0A=20=
static=20StringInfo=20failed_tests=20=3D=20NULL;=0A=20static=20bool=20=
in_note=20=3D=20false;=0A+static=20_stringlist=20*globals_before=20=3D=20=
NULL;=0A+static=20_stringlist=20*globals_after=20=3D=20NULL;=0A=20=0A=20=
static=20_resultmap=20*resultmap=20=3D=20NULL;=0A=20=0A@@=20-244,6=20=
+246,60=20@@=20split_to_stringlist(const=20char=20*s,=20const=20char=20=
*delim,=20_stringlist=20**listhead)=0A=20=09free(sc);=0A=20}=0A=20=0A+/*=0A=
+=20*=20Query=20the=20cluster=20and=20save=20the=20first=20column=20of=20=
the=20results=20into=20a=0A+=20*=20stringlist.=0A+=20*/=0A+static=20void=0A=
+query_to_stringlist(const=20char=20*dbname,=20const=20char=20*query,=20=
_stringlist=20**listhead)=0A+{=0A+=09PGconn=09=20=20=20*conn;=0A+=09=
PGresult=20=20=20*res;=0A+=09StringInfo=09buf=20=3D=20makeStringInfo();=0A=
+=0A+=09appendStringInfo(buf,=20"dbname=3D%s",=20dbname);=0A+=0A+=09conn=20=
=3D=20PQconnectdb(buf->data);=0A+=09if=20(PQstatus(conn)=20!=3D=20=
CONNECTION_OK)=0A+=09=09bail("unable=20to=20connect=20to=20cluster:=20=
%s",=20PQerrorMessage(conn));=0A+=0A+=09res=20=3D=20PQexec(conn,=20=
query);=0A+=09if=20(PQresultStatus(res)=20!=3D=20PGRES_TUPLES_OK)=0A+=09=09=
bail("unable=20to=20query=20database:=20%s",=20PQerrorMessage(conn));=0A=
+=0A+=09for=20(int=20i=20=3D=200;=20i=20<=20PQntuples(res);=20i++)=0A+=09=
=09add_stringlist_item(listhead,=20pstrdup(PQgetvalue(res,=20i,=200)));=0A=
+=0A+=09pfree(buf->data);=0A+=09PQclear(res);=0A+=09PQfinish(conn);=0A+}=0A=
+=0A+/*=0A+=20*=20Compare=20two=20stringlists=20for=20string=20equality=0A=
+=20*/=0A+static=20bool=0A+diff_stringlist(_stringlist=20*a,=20=
_stringlist=20*b)=0A+{=0A+=09_stringlist=20*sla=20=3D=20a;=0A+=09=
_stringlist=20*slb=20=3D=20b;=0A+=0A+=09for=20(;=20sla=20&&=20slb;=20sla=20=
=3D=20sla->next,=20slb=20=3D=20slb->next)=0A+=09{=0A+=09=09if=20=
(strcmp(sla->str,=20slb->str)=20!=3D=200)=0A+=09=09{=0A+=09=09=09=
diag("left=20over=20global=20object=20detected:=20%s",=20slb->str);=0A+=09=
=09=09return=20false;=0A+=09=09}=0A+=09}=0A+=0A+=09/*=20If=20we=20=
reached=20the=20end=20of=20both=20lists=20at=20the=20same=20time,=20they=20=
are=20equal=20*/=0A+=09if=20(sla=20=3D=3D=20NULL=20&&=20slb=20=3D=3D=20=
NULL)=0A+=09=09return=20true;=0A+=0A+=09return=20false;=0A+}=0A+=0A=20/*=0A=
=20=20*=20Bailing=20out=20is=20for=20unrecoverable=20errors=20which=20=
prevents=20further=20testing=20to=0A=20=20*=20occur=20and=20after=20=
which=20the=20test=20run=20should=20be=20aborted.=20By=20passing=20=
noatexit=0A@@=20-2608,6=20+2664,18=20@@=20regression_main(int=20argc,=20=
char=20*argv[],=0A=20=09=09=09create_role(sl->str,=20dblist);=0A=20=09}=0A=
=20=0A+=09/*=0A+=09=20*=20Store=20the=20global=20objects=20before=20the=20=
test=20starts=20such=20that=20we=20can=20check=0A+=09=20*=20for=20any=20=
objects=20left=20behind=20after=20the=20tests=20finish.=0A+=09=20*/=0A+=09=
query_to_stringlist("postgres",=0A+=09=09=09=09=09=09"(SELECT=20rolname=20=
AS=20obj=20FROM=20pg_catalog.pg_roles=20ORDER=20BY=201)=20"=0A+=09=09=09=09=
=09=09"UNION=20ALL=20"=0A+=09=09=09=09=09=09"(SELECT=20spcname=20AS=20=
obj=20FROM=20pg_catalog.pg_tablespace=20ORDER=20BY=201)=20"=0A+=09=09=09=09=
=09=09"UNION=20ALL=20"=0A+=09=09=09=09=09=09"(SELECT=20subname=20AS=20=
obj=20FROM=20pg_catalog.pg_subscription=20ORDER=20BY=201)",=0A+=09=09=09=09=
=09=09&globals_before);=0A+=0A=20=09/*=0A=20=09=20*=20Ready=20to=20run=20=
the=20tests=0A=20=09=20*/=0A@@=20-2621,6=20+2689,27=20@@=20=
regression_main(int=20argc,=20char=20*argv[],=0A=20=09=09=
run_single_test(sl->str,=20startfunc,=20postfunc);=0A=20=09}=0A=20=0A+=09=
/*=0A+=09=20*=20Store=20the=20global=20objects=20again=20after=20the=20=
tests=20have=20finished=20and=0A+=09=20*=20compare=20with=20the=20list=20=
from=20before=20the=20tests.=0A+=09=20*/=0A+=09=
query_to_stringlist("postgres",=0A+=09=09=09=09=09=09"(SELECT=20rolname=20=
AS=20obj=20FROM=20pg_catalog.pg_roles=20ORDER=20BY=201)=20"=0A+=09=09=09=09=
=09=09"UNION=20ALL=20"=0A+=09=09=09=09=09=09"(SELECT=20spcname=20AS=20=
obj=20FROM=20pg_catalog.pg_tablespace=20ORDER=20BY=201)=20"=0A+=09=09=09=09=
=09=09"UNION=20ALL=20"=0A+=09=09=09=09=09=09"(SELECT=20subname=20AS=20=
obj=20FROM=20pg_catalog.pg_subscription=20ORDER=20BY=201)",=0A+=09=09=09=09=
=09=09&globals_after);=0A+=0A+=09if=20(!diff_stringlist(globals_before,=20=
globals_after))=0A+=09{=0A+=09=09test_status_failed("global_objects",=20=
0.0,=20false);=0A+=09}=0A+=09else=0A+=09{=0A+=09=09=
test_status_ok("global_objects",=200.0,=20false);=0A+=09}=0A+=0A=20=09/*=0A=
=20=09=20*=20Shut=20down=20temp=20installation's=20postmaster=0A=20=09=20=
*/=0A--=20=0A2.32.1=20(Apple=20Git-133)=0A=0A=

--Apple-Mail=_F9A21101-F27B-47B9-8C10-08E797BA3490
Content-Disposition: attachment;
	filename=v4-0001-Drop-global-objects-after-completed-test.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v4-0001-Drop-global-objects-after-completed-test.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=203ce445ef8bc9d47d8ad45757df8ca3bbc7f1aa78=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20Daniel=20Gustafsson=20=
<dgustafsson@postgresql.org>=0ADate:=20Fri,=201=20Dec=202023=2012:04:49=20=
+0100=0ASubject:=20[PATCH=20v4=201/2]=20Drop=20global=20objects=20after=20=
completed=20test=0A=0AProject=20policy=20is=20to=20not=20leave=20global=20=
objects=20behind=20after=20a=20regress=0Atest=20run.=0A---=0A=20=
.../modules/test_pg_dump/expected/test_pg_dump.out=20=20=20=20=20|=20=20=
9=20+++++++++=0A=20src/test/modules/test_pg_dump/sql/test_pg_dump.sql=20=20=
=20=20=20|=2010=20++++++++++=0A=202=20files=20changed,=2019=20=
insertions(+)=0A=0Adiff=20--git=20=
a/src/test/modules/test_pg_dump/expected/test_pg_dump.out=20=
b/src/test/modules/test_pg_dump/expected/test_pg_dump.out=0Aindex=20=
f14f3a6664..479d7fc898=20100644=0A---=20=
a/src/test/modules/test_pg_dump/expected/test_pg_dump.out=0A+++=20=
b/src/test/modules/test_pg_dump/expected/test_pg_dump.out=0A@@=20-91,3=20=
+91,12=20@@=20ALTER=20EXTENSION=20test_pg_dump=20DROP=20SERVER=20s0;=0A=20=
ALTER=20EXTENSION=20test_pg_dump=20DROP=20TABLE=20test_pg_dump_t1;=0A=20=
ALTER=20EXTENSION=20test_pg_dump=20DROP=20TYPE=20test_pg_dump_e1;=0A=20=
ALTER=20EXTENSION=20test_pg_dump=20DROP=20VIEW=20test_pg_dump_v1;=0A=
+REVOKE=20USAGE=20ON=20FOREIGN=20SERVER=20s0=20FROM=20=
regress_dump_test_role;=0A+REVOKE=20SELECT=20(c1)=20ON=20ft1=20FROM=20=
regress_dump_test_role;=0A+REVOKE=20SELECT=20ON=20ft1=20FROM=20=
regress_dump_test_role;=0A+REVOKE=20UPDATE=20ON=20test_pg_dump_mv1=20=
FROM=20regress_dump_test_role;=0A+REVOKE=20USAGE=20ON=20SCHEMA=20=
test_pg_dump_s1=20FROM=20regress_dump_test_role;=0A+REVOKE=20USAGE=20ON=20=
TYPE=20test_pg_dump_e1=20FROM=20regress_dump_test_role;=0A+REVOKE=20=
EXECUTE=20ON=20FUNCTION=20test_pg_dump(int)=20FROM=20=
regress_dump_test_role;=0A+DROP=20EXTENSION=20test_pg_dump;=0A+DROP=20=
ROLE=20regress_dump_test_role;=0Adiff=20--git=20=
a/src/test/modules/test_pg_dump/sql/test_pg_dump.sql=20=
b/src/test/modules/test_pg_dump/sql/test_pg_dump.sql=0Aindex=20=
a61a7c8c4c..164d6408ee=20100644=0A---=20=
a/src/test/modules/test_pg_dump/sql/test_pg_dump.sql=0A+++=20=
b/src/test/modules/test_pg_dump/sql/test_pg_dump.sql=0A@@=20-106,3=20=
+106,13=20@@=20ALTER=20EXTENSION=20test_pg_dump=20DROP=20SERVER=20s0;=0A=20=
ALTER=20EXTENSION=20test_pg_dump=20DROP=20TABLE=20test_pg_dump_t1;=0A=20=
ALTER=20EXTENSION=20test_pg_dump=20DROP=20TYPE=20test_pg_dump_e1;=0A=20=
ALTER=20EXTENSION=20test_pg_dump=20DROP=20VIEW=20test_pg_dump_v1;=0A+=0A=
+REVOKE=20USAGE=20ON=20FOREIGN=20SERVER=20s0=20FROM=20=
regress_dump_test_role;=0A+REVOKE=20SELECT=20(c1)=20ON=20ft1=20FROM=20=
regress_dump_test_role;=0A+REVOKE=20SELECT=20ON=20ft1=20FROM=20=
regress_dump_test_role;=0A+REVOKE=20UPDATE=20ON=20test_pg_dump_mv1=20=
FROM=20regress_dump_test_role;=0A+REVOKE=20USAGE=20ON=20SCHEMA=20=
test_pg_dump_s1=20FROM=20regress_dump_test_role;=0A+REVOKE=20USAGE=20ON=20=
TYPE=20test_pg_dump_e1=20FROM=20regress_dump_test_role;=0A+REVOKE=20=
EXECUTE=20ON=20FUNCTION=20test_pg_dump(int)=20FROM=20=
regress_dump_test_role;=0A+DROP=20EXTENSION=20test_pg_dump;=0A+DROP=20=
ROLE=20regress_dump_test_role;=0A--=20=0A2.32.1=20(Apple=20Git-133)=0A=0A=

--Apple-Mail=_F9A21101-F27B-47B9-8C10-08E797BA3490--