pgsql: Improve snprintf.c's handling of NaN, Infinity, and minus zero. - Mailing list pgsql-committers

From Tom Lane
Subject pgsql: Improve snprintf.c's handling of NaN, Infinity, and minus zero.
Date
Msg-id E1g9YFj-0004Sj-Qq@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
Improve snprintf.c's handling of NaN, Infinity, and minus zero.

Up to now, float4out/float8out handled NaN and Infinity cases explicitly,
and invoked psprintf only for ordinary float values.  This was done because
platform implementations of snprintf produce varying representations of
these special cases.  But now that we use snprintf.c always, it's better
to give it the responsibility to produce a uniform representation of
these cases, so that we have uniformity across the board not only in
float4out/float8out.  Hence, move that work into fmtfloat().

Also, teach fmtfloat() to recognize IEEE minus zero and handle it
correctly.  The previous coding worked only accidentally, and would
fail for e.g. "%+f" format (it'd print "+-0.00000").  Now that we're
using snprintf.c everywhere, it's not acceptable for it to do weird
things in corner cases.  (This incidentally avoids a portability
problem we've seen on some really ancient platforms, that native
sprintf does the wrong thing with minus zero.)

Also, introduce a new entry point in snprintf.c to allow float[48]out
to bypass the work of interpreting a well-known format spec, as well
as bypassing the overhead of the psprintf layer.  I modeled this API
loosely on strfromd().  In my testing, this brings float[48]out back
to approximately the same speed they had when using native snprintf,
fixing one of the main performance issues caused by using snprintf.c.

(There is some talk of more aggressive work to improve the speed of
floating-point output conversion, but these changes seem to provide
a better starting point for such work anyway.)

Getting rid of the previous ad-hoc hack for Infinity/NaN in fmtfloat()
allows removing <ctype.h> from snprintf.c's #includes.  I also removed
a few other #includes that I think are historical, though the buildfarm
may expose that as wrong.

Discussion: https://postgr.es/m/13178.1538794717@sss.pgh.pa.us

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/6eb3eb577d76b3f58a98f78232af9e86624ce5eb

Modified Files
--------------
src/backend/utils/adt/float.c |  52 ++------------
src/include/port.h            |   3 +
src/port/snprintf.c           | 164 ++++++++++++++++++++++++++++++++++--------
3 files changed, 145 insertions(+), 74 deletions(-)


pgsql-committers by date:

Previous
From: Tom Lane
Date:
Subject: pgsql: Avoid O(N^2) cost in ExecFindRowMark().
Next
From: Alvaro Herrera
Date:
Subject: pgsql: Silence compiler warning in Assert()