> > > > Yeah, I can't get excited about the cost of that for normal user > dumps. The 002_pg_dump test does run a dump with --schema pg_catalog, > but it's dubious that that test is worth its cycles. > > Attached is the first optimization, which gets rid of the pg_class queries entirely, instead getting the same information from the existing queries in getTables and getIndexes.
Additionally, the string representation of the server version number is now stored in the Archive struct. Yes, we already have remoteVersion, but that's in integer form, and remoteVersionStr is "18devel" rather than "180000". I didn't include any work on the attribute query as I wanted to keep that separate for clarity purposes.
From 45467a69813cbf25c2850b254c5d2710c231a723 Mon Sep 17 00:00:00 2001 From: Corey Huinker <corey.huin...@gmail.com> Date: Fri, 21 Feb 2025 23:31:04 -0500 Subject: [PATCH v1] Leverage existing functions for relation stats. Rather than query pg_class once per relation in order to fetch relation stats (reltuples, relpages, relallvisible), instead just add those fields to the existing queries in getTables and getIndexes, and then store their string representations in RelStatsInfo. Additionally, we will need the string representation of the server version number so render that one and store in the Archive struct. No modification has been made for the attribute query, that will be addressed in a later patch. --- src/bin/pg_dump/pg_backup.h | 1 + src/bin/pg_dump/pg_backup_db.c | 1 + src/bin/pg_dump/pg_dump.c | 113 +++++++++++++-------------------- src/bin/pg_dump/pg_dump.h | 9 +++ 4 files changed, 54 insertions(+), 70 deletions(-) diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h index 350cf659c41..bbca3419b45 100644 --- a/src/bin/pg_dump/pg_backup.h +++ b/src/bin/pg_dump/pg_backup.h @@ -224,6 +224,7 @@ typedef struct Archive int verbose; char *remoteVersionStr; /* server's version string */ int remoteVersion; /* same in numeric form */ + char remoteVersionNumStr[32]; /* server version number, as string */ bool isStandby; /* is server a standby node */ int minRemoteVersion; /* allowable range */ diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c index 71c55d2466a..59c7b70d90f 100644 --- a/src/bin/pg_dump/pg_backup_db.c +++ b/src/bin/pg_dump/pg_backup_db.c @@ -41,6 +41,7 @@ _check_database_version(ArchiveHandle *AH) AH->public.remoteVersionStr = pg_strdup(remoteversion_str); AH->public.remoteVersion = remoteversion; + sprintf(AH->public.remoteVersionNumStr, "%d", remoteversion); if (!AH->archiveRemoteVersion) AH->archiveRemoteVersion = AH->public.remoteVersionStr; diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index afd79287177..4311ed5c65d 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -6814,7 +6814,8 @@ getFuncs(Archive *fout) * */ static RelStatsInfo * -getRelationStatistics(Archive *fout, DumpableObject *rel, char relkind) +getRelationStatistics(Archive *fout, DumpableObject *rel, const char *relpages, + const char *reltuples, const char *relallvisible, char relkind) { if (!fout->dopt->dumpStatistics) return NULL; @@ -6839,6 +6840,9 @@ getRelationStatistics(Archive *fout, DumpableObject *rel, char relkind) dobj->components |= DUMP_COMPONENT_STATISTICS; dobj->name = pg_strdup(rel->name); dobj->namespace = rel->namespace; + info->relpages = relpages; + info->reltuples = reltuples; + info->relallvisible = relallvisible; info->relkind = relkind; info->postponed_def = false; @@ -6874,6 +6878,8 @@ getTables(Archive *fout, int *numTables) int i_relhasindex; int i_relhasrules; int i_relpages; + int i_reltuples; + int i_relallvisible; int i_toastpages; int i_owning_tab; int i_owning_col; @@ -6921,6 +6927,7 @@ getTables(Archive *fout, int *numTables) appendPQExpBufferStr(query, "SELECT c.tableoid, c.oid, c.relname, " "c.relnamespace, c.relkind, c.reltype, " + "c.relpages, c.reltuples, c.relallvisible, " "c.relowner, " "c.relchecks, " "c.relhasindex, c.relhasrules, c.relpages, " @@ -7088,6 +7095,8 @@ getTables(Archive *fout, int *numTables) i_relhasindex = PQfnumber(res, "relhasindex"); i_relhasrules = PQfnumber(res, "relhasrules"); i_relpages = PQfnumber(res, "relpages"); + i_reltuples = PQfnumber(res, "reltuples"); + i_relallvisible = PQfnumber(res, "relallvisible"); i_toastpages = PQfnumber(res, "toastpages"); i_owning_tab = PQfnumber(res, "owning_tab"); i_owning_col = PQfnumber(res, "owning_col"); @@ -7151,7 +7160,10 @@ getTables(Archive *fout, int *numTables) tblinfo[i].ncheck = atoi(PQgetvalue(res, i, i_relchecks)); tblinfo[i].hasindex = (strcmp(PQgetvalue(res, i, i_relhasindex), "t") == 0); tblinfo[i].hasrules = (strcmp(PQgetvalue(res, i, i_relhasrules), "t") == 0); - tblinfo[i].relpages = atoi(PQgetvalue(res, i, i_relpages)); + tblinfo[i].relpages_s = pg_strdup(PQgetvalue(res, i, i_relpages)); + tblinfo[i].relpages = atoi(tblinfo[i].relpages_s); + tblinfo[i].reltuples_s = pg_strdup(PQgetvalue(res, i, i_reltuples)); + tblinfo[i].relallvisible_s = pg_strdup(PQgetvalue(res, i, i_relallvisible)); if (PQgetisnull(res, i, i_toastpages)) tblinfo[i].toastpages = 0; else @@ -7233,7 +7245,9 @@ getTables(Archive *fout, int *numTables) /* Add statistics */ if (tblinfo[i].interesting) - getRelationStatistics(fout, &tblinfo[i].dobj, tblinfo[i].relkind); + getRelationStatistics(fout, &tblinfo[i].dobj, tblinfo[i].relpages_s, + tblinfo[i].reltuples_s, tblinfo[i].relallvisible_s, + tblinfo[i].relkind); /* * Read-lock target tables to make sure they aren't DROPPED or altered @@ -7499,6 +7513,9 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables) i_oid, i_indrelid, i_indexname, + i_relpages, + i_reltuples, + i_relallvisible, i_parentidx, i_indexdef, i_indnkeyatts, @@ -7552,6 +7569,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables) appendPQExpBufferStr(query, "SELECT t.tableoid, t.oid, i.indrelid, " "t.relname AS indexname, " + "t.relpages, t.reltuples, t.relallvisible, " "pg_catalog.pg_get_indexdef(i.indexrelid) AS indexdef, " "i.indkey, i.indisclustered, " "c.contype, c.conname, " @@ -7659,6 +7677,9 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables) i_oid = PQfnumber(res, "oid"); i_indrelid = PQfnumber(res, "indrelid"); i_indexname = PQfnumber(res, "indexname"); + i_relpages = PQfnumber(res, "relpages"); + i_reltuples = PQfnumber(res, "reltuples"); + i_relallvisible = PQfnumber(res, "relallvisible"); i_parentidx = PQfnumber(res, "parentidx"); i_indexdef = PQfnumber(res, "indexdef"); i_indnkeyatts = PQfnumber(res, "indnkeyatts"); @@ -7732,6 +7753,9 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables) AssignDumpId(&indxinfo[j].dobj); indxinfo[j].dobj.dump = tbinfo->dobj.dump; indxinfo[j].dobj.name = pg_strdup(PQgetvalue(res, j, i_indexname)); + indxinfo[j].relpages_s = pg_strdup(PQgetvalue(res, j, i_relpages)); + indxinfo[j].reltuples_s = pg_strdup(PQgetvalue(res, j, i_reltuples)); + indxinfo[j].relallvisible_s = pg_strdup(PQgetvalue(res, j, i_relallvisible)); indxinfo[j].dobj.namespace = tbinfo->dobj.namespace; indxinfo[j].indextable = tbinfo; indxinfo[j].indexdef = pg_strdup(PQgetvalue(res, j, i_indexdef)); @@ -7759,7 +7783,9 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables) indexkind = RELKIND_PARTITIONED_INDEX; contype = *(PQgetvalue(res, j, i_contype)); - relstats = getRelationStatistics(fout, &indxinfo[j].dobj, indexkind); + relstats = getRelationStatistics(fout, &indxinfo[j].dobj, indxinfo[j].relpages_s, + indxinfo[j].reltuples_s, indxinfo[j].relallvisible_s, + indexkind); if (contype == 'p' || contype == 'u' || contype == 'x') { @@ -10383,18 +10409,6 @@ dumpComment(Archive *fout, const char *type, catalogId, subid, dumpId, NULL); } -/* - * Tabular description of the parameters to pg_restore_relation_stats() - * param_name, param_type - */ -static const char *rel_stats_arginfo[][2] = { - {"relation", "regclass"}, - {"version", "integer"}, - {"relpages", "integer"}, - {"reltuples", "real"}, - {"relallvisible", "integer"}, -}; - /* * Tabular description of the parameters to pg_restore_attribute_stats() * param_name, param_type @@ -10419,30 +10433,6 @@ static const char *att_stats_arginfo[][2] = { {"range_bounds_histogram", "text"}, }; -/* - * getRelStatsExportQuery -- - * - * Generate a query that will fetch all relation (e.g. pg_class) - * stats for a given relation. - */ -static void -getRelStatsExportQuery(PQExpBuffer query, Archive *fout, - const char *schemaname, const char *relname) -{ - resetPQExpBuffer(query); - appendPQExpBufferStr(query, - "SELECT c.oid::regclass AS relation, " - "current_setting('server_version_num') AS version, " - "c.relpages, c.reltuples, c.relallvisible " - "FROM pg_class c " - "JOIN pg_namespace n " - "ON n.oid = c.relnamespace " - "WHERE n.nspname = "); - appendStringLiteralAH(query, schemaname, fout); - appendPQExpBufferStr(query, " AND c.relname = "); - appendStringLiteralAH(query, relname, fout); -} - /* * getAttStatsExportQuery -- * @@ -10521,33 +10511,20 @@ appendNamedArgument(PQExpBuffer out, Archive *fout, const char *argname, * Append a formatted pg_restore_relation_stats statement. */ static void -appendRelStatsImport(PQExpBuffer out, Archive *fout, PGresult *res) +appendRelStatsImport(PQExpBuffer out, Archive *fout, const RelStatsInfo *rsinfo) { - const char *sep = ""; - - if (PQntuples(res) == 0) - return; + const char *qualname = fmtQualifiedId(rsinfo->dobj.namespace->dobj.name, rsinfo->dobj.name); appendPQExpBufferStr(out, "SELECT * FROM pg_catalog.pg_restore_relation_stats(\n"); - - for (int argno = 0; argno < lengthof(rel_stats_arginfo); argno++) - { - const char *argname = rel_stats_arginfo[argno][0]; - const char *argtype = rel_stats_arginfo[argno][1]; - int fieldno = PQfnumber(res, argname); - - if (fieldno < 0) - pg_fatal("relation stats export query missing field '%s'", - argname); - - if (PQgetisnull(res, 0, fieldno)) - continue; - - appendPQExpBufferStr(out, sep); - appendNamedArgument(out, fout, argname, PQgetvalue(res, 0, fieldno), argtype); - - sep = ",\n"; - } + appendNamedArgument(out, fout, "relation", qualname, "regclass"); + appendPQExpBufferStr(out, ",\n"); + appendNamedArgument(out, fout, "version", fout->remoteVersionNumStr, "integer"); + appendPQExpBufferStr(out, ",\n"); + appendNamedArgument(out, fout, "relpages", rsinfo->relpages, "integer"); + appendPQExpBufferStr(out, ",\n"); + appendNamedArgument(out, fout, "reltuples", rsinfo->reltuples, "real"); + appendPQExpBufferStr(out, ",\n"); + appendNamedArgument(out, fout, "relallvisible", rsinfo->relallvisible, "integer"); appendPQExpBufferStr(out, "\n);\n"); } @@ -10643,15 +10620,11 @@ dumpRelationStats(Archive *fout, const RelStatsInfo *rsinfo) tag = createPQExpBuffer(); appendPQExpBufferStr(tag, fmtId(dobj->name)); - query = createPQExpBuffer(); out = createPQExpBuffer(); - getRelStatsExportQuery(query, fout, dobj->namespace->dobj.name, - dobj->name); - res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK); - appendRelStatsImport(out, fout, res); - PQclear(res); + appendRelStatsImport(out, fout, rsinfo); + query = createPQExpBuffer(); getAttStatsExportQuery(query, fout, dobj->namespace->dobj.name, dobj->name); res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK); diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index f08f5905aa3..1fd1812d348 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -329,6 +329,9 @@ typedef struct _tableInfo int owning_col; /* attr # of column owning sequence */ bool is_identity_sequence; int relpages; /* table's size in pages (from pg_class) */ + const char *relpages_s; /* table's size in pages (from pg_class) */ + const char *reltuples_s; /* table's approx number of tuples (from pg_class) */ + const char *relallvisible_s; /* table's number of pages all-visible (from pg_class) */ int toastpages; /* toast table's size in pages, if any */ bool interesting; /* true if need to collect more data */ @@ -418,6 +421,9 @@ typedef struct _indxInfo int indnattrs; /* total number of index attributes */ Oid *indkeys; /* In spite of the name 'indkeys' this field * contains both key and nonkey attributes */ + const char *relpages_s; + const char *reltuples_s; + const char *relallvisible_s; bool indisclustered; bool indisreplident; bool indnullsnotdistinct; @@ -438,6 +444,9 @@ typedef struct _indexAttachInfo typedef struct _relStatsInfo { DumpableObject dobj; + const char *relpages; + const char *reltuples; + const char *relallvisible; char relkind; /* 'r', 'm', 'i', etc */ bool postponed_def; /* stats must be postponed into post-data */ } RelStatsInfo; base-commit: f8d7f29b3e81db59b95e4b5baaa6943178c89fd8 -- 2.48.1