Skip to content

Commit f397e08

Browse files
RhodiumToadRhodiumToad
committed
Use strtof() and not strtod() for float4 input.
Using strtod() creates a double-rounding problem; the input decimal value is first rounded to the nearest double; rounding that to the nearest float may then give an incorrect result. An example is that 7.038531e-26 when input via strtod and then rounded to float4 gives 0xAE43FEp-107 instead of the correct 0xAE43FDp-107. Values output by earlier PG versions with extra_float_digits=3 should all be read in with the same values as previously. However, values supplied by other software using shortest representations could be mis-read. On platforms that lack a strtof() entirely, we fall back to the old incorrect rounding behavior. (As strtof() is required by C99, such platforms are considered of primarily historical interest.) On VS2013, some workarounds are used to get correct error handling. The regression tests now test for the correct input values, so platforms that lack strtof() will need resultmap entries. An entry for HP-UX 10 is included (more may be needed). Reviewed-By: Tom Lane Discussion: https://postgr.es/m/871s5emitx.fsf@news-spur.riddles.org.uk Discussion: https://postgr.es/m/87d0owlqpv.fsf@news-spur.riddles.org.uk
1 parent 37d9916 commit f397e08

File tree

13 files changed

+810
-17
lines changed

13 files changed

+810
-17
lines changed

‎configure

+13
Original file line numberDiff line numberDiff line change
@@ -15805,6 +15805,19 @@ esac
1580515805

1580615806
fi
1580715807

15808+
ac_fn_c_check_func "$LINENO" "strtof" "ac_cv_func_strtof"
15809+
if test "x$ac_cv_func_strtof" = xyes; then :
15810+
$as_echo "#define HAVE_STRTOF 1" >>confdefs.h
15811+
15812+
else
15813+
case " $LIBOBJS " in
15814+
*" strtof.$ac_objext "* ) ;;
15815+
*) LIBOBJS="$LIBOBJS strtof.$ac_objext"
15816+
;;
15817+
esac
15818+
15819+
fi
15820+
1580815821

1580915822

1581015823
case $host_os in

‎configure.in

+1
Original file line numberDiff line numberDiff line change
@@ -1705,6 +1705,7 @@ AC_REPLACE_FUNCS(m4_normalize([
17051705
strlcat
17061706
strlcpy
17071707
strnlen
1708+
strtof
17081709
]))
17091710

17101711
case $host_os in

‎src/backend/utils/adt/float.c

+43-13
Original file line numberDiff line numberDiff line change
@@ -104,13 +104,39 @@ is_infinite(double val)
104104

105105
/*
106106
* float4in - converts "num" to float4
107+
*
108+
* Note that this code now uses strtof(), where it used to use strtod().
109+
*
110+
* The motivation for using strtof() is to avoid a double-rounding problem:
111+
* for certain decimal inputs, if you round the input correctly to a double,
112+
* and then round the double to a float, the result is incorrect in that it
113+
* does not match the result of rounding the decimal value to float directly.
114+
*
115+
* One of the best examples is 7.038531e-26:
116+
*
117+
* 0xAE43FDp-107 = 7.03853069185120912085...e-26
118+
* midpoint 7.03853100000000022281...e-26
119+
* 0xAE43FEp-107 = 7.03853130814879132477...e-26
120+
*
121+
* making 0xAE43FDp-107 the correct float result, but if you do the conversion
122+
* via a double, you get
123+
*
124+
* 0xAE43FD.7FFFFFF8p-107 = 7.03853099999999907487...e-26
125+
* midpoint 7.03853099999999964884...e-26
126+
* 0xAE43FD.80000000p-107 = 7.03853100000000022281...e-26
127+
* 0xAE43FD.80000008p-107 = 7.03853100000000137076...e-26
128+
*
129+
* so the value rounds to the double exactly on the midpoint between the two
130+
* nearest floats, and then rounding again to a float gives the incorrect
131+
* result of 0xAE43FEp-107.
132+
*
107133
*/
108134
Datum
109135
float4in(PG_FUNCTION_ARGS)
110136
{
111137
char *num = PG_GETARG_CSTRING(0);
112138
char *orig_num;
113-
double val;
139+
float val;
114140
char *endptr;
115141

116142
/*
@@ -135,22 +161,22 @@ float4in(PG_FUNCTION_ARGS)
135161
"real", orig_num)));
136162

137163
errno = 0;
138-
val = strtod(num, &endptr);
164+
val = strtof(num, &endptr);
139165

140166
/* did we not see anything that looks like a double? */
141167
if (endptr == num || errno != 0)
142168
{
143169
int save_errno = errno;
144170

145171
/*
146-
* C99 requires that strtod() accept NaN, [+-]Infinity, and [+-]Inf,
172+
* C99 requires that strtof() accept NaN, [+-]Infinity, and [+-]Inf,
147173
* but not all platforms support all of these (and some accept them
148174
* but set ERANGE anyway...) Therefore, we check for these inputs
149-
* ourselves if strtod() fails.
175+
* ourselves if strtof() fails.
150176
*
151177
* Note: C99 also requires hexadecimal input as well as some extended
152178
* forms of NaN, but we consider these forms unportable and don't try
153-
* to support them. You can use 'em if your strtod() takes 'em.
179+
* to support them. You can use 'em if your strtof() takes 'em.
154180
*/
155181
if (pg_strncasecmp(num, "NaN", 3) == 0)
156182
{
@@ -195,8 +221,18 @@ float4in(PG_FUNCTION_ARGS)
195221
* precision). We'd prefer not to throw error for that, so try to
196222
* detect whether it's a "real" out-of-range condition by checking
197223
* to see if the result is zero or huge.
224+
*
225+
* Use isinf() rather than HUGE_VALF on VS2013 because it generates
226+
* a spurious overflow warning for -HUGE_VALF. Also use isinf() if
227+
* HUGE_VALF is missing.
198228
*/
199-
if (val == 0.0 || val >= HUGE_VAL || val <= -HUGE_VAL)
229+
if (val == 0.0 ||
230+
#if !defined(HUGE_VALF) || (defined(_MSC_VER) && (_MSC_VER < 1900))
231+
isinf(val)
232+
#else
233+
(val >= HUGE_VALF || val <= -HUGE_VALF)
234+
#endif
235+
)
200236
ereport(ERROR,
201237
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
202238
errmsg("\"%s\" is out of range for type real",
@@ -232,13 +268,7 @@ float4in(PG_FUNCTION_ARGS)
232268
errmsg("invalid input syntax for type %s: \"%s\"",
233269
"real", orig_num)));
234270

235-
/*
236-
* if we get here, we have a legal double, still need to check to see if
237-
* it's a legal float4
238-
*/
239-
check_float4_val((float4) val, isinf(val), val == 0);
240-
241-
PG_RETURN_FLOAT4((float4) val);
271+
PG_RETURN_FLOAT4(val);
242272
}
243273

244274
/*

‎src/include/pg_config.h.in

+3
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,9 @@
555555
/* Define to 1 if you have the `strsignal' function. */
556556
#undef HAVE_STRSIGNAL
557557

558+
/* Define to 1 if you have the `strtof' function. */
559+
#undef HAVE_STRTOF
560+
558561
/* Define to 1 if you have the `strtoll' function. */
559562
#undef HAVE_STRTOLL
560563

‎src/include/pg_config.h.win32

+3
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,9 @@
139139
don't. */
140140
#define HAVE_DECL_STRNLEN 1
141141

142+
/* Define to 1 if you have the `strtof' function. */
143+
#define HAVE_STRTOF 1
144+
142145
/* Define to 1 if you have the declaration of `strtoll', and to 0 if you
143146
don't. */
144147
#define HAVE_DECL_STRTOLL 1

‎src/include/port.h

+4
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,10 @@ extern int isinf(double x);
381381
#endif /* __clang__ && !__cplusplus */
382382
#endif /* !HAVE_ISINF */
383383

384+
#ifndef HAVE_STRTOF
385+
extern float strtof(const char *nptr, char **endptr);
386+
#endif
387+
384388
#ifndef HAVE_MKDTEMP
385389
extern char *mkdtemp(char *path);
386390
#endif

‎src/include/port/win32_port.h

+12
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,18 @@ typedef unsigned short mode_t;
510510
#define isnan(x) _isnan(x)
511511
#endif
512512

513+
#if defined(_MSC_VER) && (_MSC_VER < 1900)
514+
/*
515+
* VS2013 has a strtof() that seems to give correct answers for valid input,
516+
* even on the rounding edge cases, but which doesn't handle out-of-range
517+
* input correctly. Work around that.
518+
*/
519+
#define HAVE_BUGGY_WINDOWS_STRTOF 1
520+
extern float pg_strtof(const char *nptr, char **endptr);
521+
#define strtof(a,b) (pg_strtof((a),(b)))
522+
523+
#endif
524+
513525
/* Pulled from Makefile.port in MinGW */
514526
#define DLSUFFIX ".dll"
515527

‎src/port/strtof.c

+123
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
/*-------------------------------------------------------------------------
2+
*
3+
* strtof.c
4+
*
5+
* Portions Copyright (c) 2019, PostgreSQL Global Development Group
6+
*
7+
*
8+
* IDENTIFICATION
9+
* src/port/strtof.c
10+
*
11+
*-------------------------------------------------------------------------
12+
*/
13+
14+
#include "c.h"
15+
16+
#include <float.h>
17+
#include <math.h>
18+
19+
#ifndef HAVE_STRTOF
20+
/*
21+
* strtof() is part of C99; this version is only for the benefit of obsolete
22+
* platforms. As such, it is known to return incorrect values for edge cases,
23+
* which have to be allowed for in variant files for regression test results
24+
* for any such platform.
25+
*/
26+
27+
float
28+
strtof(const char *nptr, char **endptr)
29+
{
30+
int caller_errno = errno;
31+
double dresult;
32+
float fresult;
33+
34+
errno = 0;
35+
dresult = strtod(nptr, endptr);
36+
fresult = (float) dresult;
37+
38+
if (errno == 0)
39+
{
40+
/*
41+
* Value might be in-range for double but not float.
42+
*/
43+
if (dresult != 0 && fresult == 0)
44+
caller_errno = ERANGE; /* underflow */
45+
if (!isinf(dresult) && isinf(fresult))
46+
caller_errno = ERANGE; /* overflow */
47+
}
48+
else
49+
caller_errno = errno;
50+
51+
errno = caller_errno;
52+
return fresult;
53+
}
54+
55+
#elif HAVE_BUGGY_WINDOWS_STRTOF
56+
/*
57+
* On Windows, there's a slightly different problem: VS2013 has a strtof()
58+
* that returns the correct results for valid input, but may fail to report an
59+
* error for underflow or overflow, returning 0 instead. Work around that by
60+
* trying strtod() when strtof() returns 0.0 or [+-]Inf, and calling it an
61+
* error if the result differs. Also, strtof() doesn't handle subnormal input
62+
* well, so prefer to round the strtod() result in such cases. (Normally we'd
63+
* just say "too bad" if strtof() doesn't support subnormals, but since we're
64+
* already in here fixing stuff, we might as well do the best fix we can.)
65+
*/
66+
float
67+
pg_strtof(const char *nptr, char **endptr)
68+
{
69+
int caller_errno = errno;
70+
float fresult;
71+
72+
errno = 0;
73+
fresult = (strtof)(nptr, endptr);
74+
if (errno)
75+
{
76+
/* On error, just return the error to the caller. */
77+
return fresult;
78+
}
79+
else if ((*endptr == nptr) || isnan(fresult) ||
80+
((fresult >= FLT_MIN || fresult <= -FLT_MIN) && !isinf(fresult)))
81+
{
82+
/*
83+
* If we got nothing parseable, or if we got a non-0 non-subnormal
84+
* finite value (or NaN) without error, then return that to the caller
85+
* without error.
86+
*/
87+
errno = caller_errno;
88+
return fresult;
89+
}
90+
else
91+
{
92+
/*
93+
* Try again. errno is already 0 here.
94+
*/
95+
double dresult = strtod(nptr, NULL);
96+
if (errno)
97+
{
98+
/* On error, just return the error */
99+
return fresult;
100+
}
101+
else if ((dresult == 0.0 && fresult == 0.0) ||
102+
(isinf(dresult) && isinf(fresult) && (fresult == dresult)))
103+
{
104+
/* both values are 0 or infinities of the same sign */
105+
errno = caller_errno;
106+
return fresult;
107+
}
108+
else if ((dresult > 0 && dresult <= FLT_MIN && (float)dresult != 0.0) ||
109+
(dresult < 0 && dresult >= -FLT_MIN && (float)dresult != 0.0))
110+
{
111+
/* subnormal but nonzero value */
112+
errno = caller_errno;
113+
return (float) dresult;
114+
}
115+
else
116+
{
117+
errno = ERANGE;
118+
return fresult;
119+
}
120+
}
121+
}
122+
123+
#endif

0 commit comments

Comments
 (0)