Don't convert to and from floats in pg_dump.

Commit 8f427187db improved performance by remembering relation stats
as native types rather than issuing a new query for each relation.

Using native types is fine for integers like relpages; but reltuples
is floating point. The commit controllled for that complexity by using
setlocale(LC_NUMERIC, "C"). After that, Alexander Lakhin found a
problem in pg_strtof(), fixed in 00d61a08c5.

While we aren't aware of any more problems with that approach, it
seems wise to just use a string the whole way for floating point
values, as Corey's original patch did, and get rid of the
setlocale(). Integers are still converted to native types to avoid
wasting memory.

Co-authored-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://postgr.es/m/3049348.1740855411@sss.pgh.pa.us
Discussion: https://postgr.es/m/560cca3781740bd69881bb07e26eb8f65b09792c.camel%40j-davis.com

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 4f4ad2ee15070a963ae8c05882f38b59e0dde3a9..c371570501ac0f5bece8e4adbc79f50f3dfbbec0 100644 (file)
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -525,9 +525,6 @@ main(int argc, char **argv)
    pg_logging_set_level(PG_LOG_WARNING);
    set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_dump"));
 
-   /* ensure that locale does not affect floating point interpretation */
-   setlocale(LC_NUMERIC, "C");
-
    /*
     * Initialize what we need for parallel execution, especially for thread
     * support on Windows.
@@ -6816,10 +6813,12 @@ getFuncs(Archive *fout)
  * getRelationStatistics
  *    register the statistics object as a dependent of the relation.
  *
+ * reltuples is passed as a string to avoid complexities in converting from/to
+ * floating point.
  */
 static RelStatsInfo *
 getRelationStatistics(Archive *fout, DumpableObject *rel, int32 relpages,
-                     float reltuples, int32 relallvisible, char relkind,
+                     char *reltuples, int32 relallvisible, char relkind,
                      char **indAttNames, int nindAttNames)
 {
    if (!fout->dopt->dumpStatistics)
@@ -6846,7 +6845,7 @@ getRelationStatistics(Archive *fout, DumpableObject *rel, int32 relpages,
        dobj->name = pg_strdup(rel->name);
        dobj->namespace = rel->namespace;
        info->relpages = relpages;
-       info->reltuples = reltuples;
+       info->reltuples = pstrdup(reltuples);
        info->relallvisible = relallvisible;
        info->relkind = relkind;
        info->indAttNames = indAttNames;
@@ -7149,7 +7148,6 @@ getTables(Archive *fout, int *numTables)
 
    for (i = 0; i < ntups; i++)
    {
-       float       reltuples = strtof(PQgetvalue(res, i, i_reltuples), NULL);
        int32       relallvisible = atoi(PQgetvalue(res, i, i_relallvisible));
 
        tblinfo[i].dobj.objType = DO_TABLE;
@@ -7252,8 +7250,8 @@ getTables(Archive *fout, int *numTables)
        /* Add statistics */
        if (tblinfo[i].interesting)
            getRelationStatistics(fout, &tblinfo[i].dobj, tblinfo[i].relpages,
-                                 reltuples, relallvisible, tblinfo[i].relkind,
-                                 NULL, 0);
+                                 PQgetvalue(res, i, i_reltuples),
+                                 relallvisible, tblinfo[i].relkind, NULL, 0);
 
        /*
         * Read-lock target tables to make sure they aren't DROPPED or altered
@@ -7762,7 +7760,6 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
            char        indexkind;
            RelStatsInfo *relstats;
            int32       relpages = atoi(PQgetvalue(res, j, i_relpages));
-           float       reltuples = strtof(PQgetvalue(res, j, i_reltuples), NULL);
            int32       relallvisible = atoi(PQgetvalue(res, j, i_relallvisible));
 
            indxinfo[j].dobj.objType = DO_INDEX;
@@ -7805,7 +7802,8 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
            }
 
            relstats = getRelationStatistics(fout, &indxinfo[j].dobj, relpages,
-                                            reltuples, relallvisible, indexkind,
+                                            PQgetvalue(res, j, i_reltuples),
+                                            relallvisible, indexkind,
                                             indAttNames, nindAttNames);
 
            contype = *(PQgetvalue(res, j, i_contype));
@@ -10493,7 +10491,6 @@ dumpRelationStats(Archive *fout, const RelStatsInfo *rsinfo)
    DumpId     *deps = NULL;
    int         ndeps = 0;
    char       *qualified_name;
-   char        reltuples_str[FLOAT_SHORTEST_DECIMAL_LEN];
    int         i_attname;
    int         i_inherited;
    int         i_null_frac;
@@ -10568,8 +10565,7 @@ dumpRelationStats(Archive *fout, const RelStatsInfo *rsinfo)
    appendStringLiteralAH(out, qualified_name, fout);
    appendPQExpBufferStr(out, "::regclass,\n");
    appendPQExpBuffer(out, "\t'relpages', '%d'::integer,\n", rsinfo->relpages);
-   float_to_shortest_decimal_buf(rsinfo->reltuples, reltuples_str);
-   appendPQExpBuffer(out, "\t'reltuples', '%s'::real,\n", reltuples_str);
+   appendPQExpBuffer(out, "\t'reltuples', '%s'::real,\n", rsinfo->reltuples);
    appendPQExpBuffer(out, "\t'relallvisible', '%d'::integer\n);\n",
                      rsinfo->relallvisible);
 

diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index ca32f167878eb8d0e5cb73747b63108e6b917110..bbdb30b5f54e56a17d5ee628c3c55b1815b81ace 100644 (file)
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -439,7 +439,7 @@ typedef struct _relStatsInfo
 {
    DumpableObject dobj;
    int32       relpages;
-   float       reltuples;
+   char       *reltuples;
    int32       relallvisible;
    char        relkind;        /* 'r', 'm', 'i', etc */