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

2 files changed, 10 insertions, 14 deletions

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 4f4ad2ee150..c371570501a 100644
--- 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 ca32f167878..bbdb30b5f54 100644
--- 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 */