Hi,
On 2025-02-24 05:11:48 -0500, Corey Huinker wrote:
> Incorporating most of the feedback (I kept a few of
> the appendNamedArgument() calls) presented over the weekend.
>
> * removeVersionNumStr is gone
> * relpages/reltuples/relallvisible are now char[32] buffers in RelStatsInfo
> and nowhere else (existing relpages conversion remains, however)
I don't see the point. This will use more memory and if we can't get
conversions between integers and strings right we have much bigger
problems. The same code was used in the backend too!
And it leads to storing relpages in two places, with different
transformations, which doesn't seem great.
> @@ -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, "
That query is already querying relpages a bit later in the query, so we'd
query the column twice.
> + printfPQExpBuffer(query, "EXECUTE getAttributeStats(");
> + appendStringLiteralAH(query, dobj->namespace->dobj.name, fout);
> + appendPQExpBufferStr(query, ", ");
> + appendStringLiteralAH(query, dobj->name, fout);
> + appendPQExpBufferStr(query, "); ");
> res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
It seems somewhat ugly that we're building an SQL string with non-trivial
constants. It'd be better to use PQexecParams() - but I guess we don't have
any uses of it yet in pg_dump.
ISTM that we ought to expose the relation oid in pg_stats. This query would be
simpler and faster if we could just use the oid as the predicate. Will take a
while till we can rely on that, but still.
Have you compared performance of with/without stats after these optimizations?
Greetings,
Andres Freund
