From: Tom Lane <tgl@sss.pgh.pa.us>
To: Laurenz Albe <laurenz.albe@cybertec.at>
cc: depesz@depesz.com, pgsql-general <pgsql-general@lists.postgresql.org>
Subject: Re: Can we get rid of repeated queries from pg_dump?
In-reply-to: <e6a7503afd115d111fbb96c5787906d73cfe2605.camel@cybertec.at>
References: <20210826084430.GA26282@depesz.com>
 <648517.1629986527@sss.pgh.pa.us> <20210826140827.GA15868@depesz.com>
 <649446.1629987629@sss.pgh.pa.us> <20210826160644.GA3120@depesz.com>
 <e6a7503afd115d111fbb96c5787906d73cfe2605.camel@cybertec.at>
Comments: In-reply-to Laurenz Albe <laurenz.albe@cybertec.at>
	message dated "Fri, 27 Aug 2021 09:33:51 +0200"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <896721.1630099383.0@sss.pgh.pa.us>
Date: Fri, 27 Aug 2021 17:23:23 -0400
Message-ID: <896732.1630099403@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/896732.1630099403%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <896721.1630099383.1@sss.pgh.pa.us>

Laurenz Albe <laurenz.albe@cybertec.at> writes:
> On Thu, 2021-08-26 at 18:06 +0200, hubert depesz lubaczewski wrote:
>> In total, there were 5000 queries:
>> SELECT pg_catalog.format_type('[0-9]+'::pg_catalog.oid, NULL)
>> But there were only 83 separate oids that were scanned.

> That is a strong argument for using a hash table to cache the types.

Those queries are coming from getFormattedTypeName(), which is used
for function arguments and the like.  I'm not quite sure why Hubert
is seeing 5000 such calls in a database with only ~100 functions;
surely they don't all have an average of 50 arguments?

I experimented with the attached, very quick-n-dirty patch to collect
format_type results during the initial scan of pg_type, instead.  On the
regression database in HEAD, it reduces the number of queries pg_dump
issues from 3260 to 2905; but I'm having a hard time detecting any net
performance change.

(This is not meant for commit as-is; notably, I didn't bother to fix
getTypes' code paths for pre-9.6 servers.  It should be fine for
performance testing though.)

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff;
	name="avoid-repeated-format_type-queries-0.1.patch";
	charset="us-ascii"
Content-ID: <896721.1630099383.2@sss.pgh.pa.us>
Content-Description: avoid-repeated-format_type-queries-0.1.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 6adbd20778..efe1be533f 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -5088,6 +5088,7 @@ getTypes(Archive *fout, int *numTypes)
 	int			i_oid;
 	int			i_typname;
 	int			i_typnamespace;
+	int			i_ftypname;
 	int			i_typacl;
 	int			i_rtypacl;
 	int			i_inittypacl;
@@ -5129,6 +5130,7 @@ getTypes(Archive *fout, int *numTypes)
 =

 		appendPQExpBuffer(query, "SELECT t.tableoid, t.oid, t.typname, "
 						  "t.typnamespace, "
+						  "pg_catalog.format_type(t.oid, NULL) AS ftypname, "
 						  "%s AS typacl, "
 						  "%s AS rtypacl, "
 						  "%s AS inittypacl, "
@@ -5211,6 +5213,7 @@ getTypes(Archive *fout, int *numTypes)
 	i_oid =3D PQfnumber(res, "oid");
 	i_typname =3D PQfnumber(res, "typname");
 	i_typnamespace =3D PQfnumber(res, "typnamespace");
+	i_ftypname =3D PQfnumber(res, "ftypname");
 	i_typacl =3D PQfnumber(res, "typacl");
 	i_rtypacl =3D PQfnumber(res, "rtypacl");
 	i_inittypacl =3D PQfnumber(res, "inittypacl");
@@ -5232,6 +5235,7 @@ getTypes(Archive *fout, int *numTypes)
 		tyinfo[i].dobj.name =3D pg_strdup(PQgetvalue(res, i, i_typname));
 		tyinfo[i].dobj.namespace =3D
 			findNamespace(atooid(PQgetvalue(res, i, i_typnamespace)));
+		tyinfo[i].ftypname =3D pg_strdup(PQgetvalue(res, i, i_ftypname));
 		tyinfo[i].rolname =3D pg_strdup(PQgetvalue(res, i, i_rolname));
 		tyinfo[i].typacl =3D pg_strdup(PQgetvalue(res, i, i_typacl));
 		tyinfo[i].rtypacl =3D pg_strdup(PQgetvalue(res, i, i_rtypacl));
@@ -18892,12 +18896,11 @@ findDumpableDependencies(ArchiveHandle *AH, cons=
t DumpableObject *dobj,
  *
  * This does not guarantee to schema-qualify the output, so it should not
  * be used to create the target object name for CREATE or ALTER commands.
- *
- * TODO: there might be some value in caching the results.
  */
 static char *
 getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
 {
+	TypeInfo   *typeInfo;
 	char	   *result;
 	PQExpBuffer query;
 	PGresult   *res;
@@ -18910,6 +18913,16 @@ getFormattedTypeName(Archive *fout, Oid oid, OidO=
ptions opts)
 			return pg_strdup("NONE");
 	}
 =

+	typeInfo =3D findTypeByOid(oid);
+	if (typeInfo)
+		return pg_strdup(typeInfo->ftypname);
+
+	/*
+	 * Fall back to asking the server.  XXX we could only reach this code if
+	 * getTypes() didn't see the type, which should never happen.  If it did
+	 * happen, it'd imply that we don't have proper dependencies for the
+	 * object we're trying to describe.  Maybe just throw an error instead?
+	 */
 	query =3D createPQExpBuffer();
 	appendPQExpBuffer(query, "SELECT pg_catalog.format_type('%u'::pg_catalog=
.oid, NULL)",
 					  oid);
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index f5e170e0db..29af845ece 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -166,9 +166,11 @@ typedef struct _typeInfo
 	DumpableObject dobj;
 =

 	/*
-	 * Note: dobj.name is the pg_type.typname entry.  format_type() might
-	 * produce something different than typname
+	 * Note: dobj.name is the raw pg_type.typname entry.  ftypname is the
+	 * result of format_type(), which will be quoted if needed, and might be
+	 * schema-qualified too.
 	 */
+	char	   *ftypname;
 	char	   *rolname;		/* name of owner, or empty string */
 	char	   *typacl;
 	char	   *rtypacl;

------- =_aaaaaaaaaa0--