Re: Can we get rid of repeated queries from pg_dump?

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

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 = PQfnumber(res, "oid");
     i_typname = PQfnumber(res, "typname");
     i_typnamespace = PQfnumber(res, "typnamespace");
+    i_ftypname = PQfnumber(res, "ftypname");
     i_typacl = PQfnumber(res, "typacl");
     i_rtypacl = PQfnumber(res, "rtypacl");
     i_inittypacl = PQfnumber(res, "inittypacl");
@@ -5232,6 +5235,7 @@ getTypes(Archive *fout, int *numTypes)
         tyinfo[i].dobj.name = pg_strdup(PQgetvalue(res, i, i_typname));
         tyinfo[i].dobj.namespace =
             findNamespace(atooid(PQgetvalue(res, i, i_typnamespace)));
+        tyinfo[i].ftypname = pg_strdup(PQgetvalue(res, i, i_ftypname));
         tyinfo[i].rolname = pg_strdup(PQgetvalue(res, i, i_rolname));
         tyinfo[i].typacl = pg_strdup(PQgetvalue(res, i, i_typacl));
         tyinfo[i].rtypacl = pg_strdup(PQgetvalue(res, i, i_rtypacl));
@@ -18892,12 +18896,11 @@ findDumpableDependencies(ArchiveHandle *AH, const 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, OidOptions opts)
             return pg_strdup("NONE");
     }

+    typeInfo = 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 = 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;