Skip to content

Commit 73193d8

Browse files
tglsfdctglsfdc
committed
Adjust degree-based trig functions for more portability.
The buildfarm isn't very happy with the results of commit e1bd684. To try to get the expected exact results everywhere: * Replace M_PI / 180 subexpressions with a precomputed constant, so that the compiler can't decide to rearrange that division with an adjacent operation. Hopefully this will fix failures to get exactly 0.5 from sind(30) and cosd(60). * Add scaling to ensure that tand(45) and cotd(45) give exactly 1; there was nothing particularly guaranteeing that before. * Replace minus zero by zero when tand() or cotd() would output that; many machines did so for tand(180) and cotd(270), but not all. We could alternatively deem both results valid, but that doesn't seem likely to be what users will want.
1 parent 6ae4c8d commit 73193d8

File tree

5 files changed

+60
-15
lines changed

5 files changed

+60
-15
lines changed

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

Lines changed: 52 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@
3131
#define M_PI 3.14159265358979323846
3232
#endif
3333

34+
/* Radians per degree, a.k.a. PI / 180 */
35+
#define RADIANS_PER_DEGREE 0.0174532925199432957692
36+
3437
/* Visual C++ etc lacks NAN, and won't accept 0.0/0.0. NAN definition from
3538
* http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vclang/html/vclrfNotNumberNANItems.asp
3639
*/
@@ -1919,7 +1922,7 @@ datan2d(PG_FUNCTION_ARGS)
19191922
static double
19201923
sind_0_to_30(double x)
19211924
{
1922-
return (sin(x * (M_PI / 180.0)) / sin(30.0 * (M_PI / 180.0))) / 2.0;
1925+
return (sin(x * RADIANS_PER_DEGREE) / sin(30.0 * RADIANS_PER_DEGREE)) / 2.0;
19231926
}
19241927

19251928

@@ -1931,8 +1934,8 @@ sind_0_to_30(double x)
19311934
static double
19321935
cosd_0_to_60(double x)
19331936
{
1934-
return 1.0 - ((1.0 - cos(x * (M_PI / 180.0))) /
1935-
(1.0 - cos(60.0 * (M_PI / 180.0)))) / 2.0;
1937+
return 1.0 - ((1.0 - cos(x * RADIANS_PER_DEGREE)) /
1938+
(1.0 - cos(60.0 * RADIANS_PER_DEGREE))) / 2.0;
19361939
}
19371940

19381941

@@ -2030,8 +2033,9 @@ Datum
20302033
dcotd(PG_FUNCTION_ARGS)
20312034
{
20322035
float8 arg1 = PG_GETARG_FLOAT8(0);
2033-
int sign = 1;
20342036
float8 result;
2037+
int sign = 1;
2038+
static float8 cot45 = 0.0;
20352039

20362040
/*
20372041
* Per the POSIX spec, return NaN if the input is NaN and throw an error
@@ -2071,6 +2075,26 @@ dcotd(PG_FUNCTION_ARGS)
20712075

20722076
result = sign * cosd_q1(arg1) / sind_q1(arg1);
20732077

2078+
/*
2079+
* We want cotd(45) to be exactly 1, but the above computation might've
2080+
* produced something different, so scale to get the right result. To
2081+
* avoid redoing cosd_q1(45) / sind_q1(45) many times, and to prevent the
2082+
* compiler from maybe rearranging the calculation, cache that value in a
2083+
* static variable.
2084+
*/
2085+
if (cot45 == 0.0)
2086+
cot45 = cosd_q1(45.0) / sind_q1(45.0);
2087+
2088+
result /= cot45;
2089+
2090+
/*
2091+
* On some machines, we get cotd(270) = minus zero, but this isn't always
2092+
* true. For portability, and because the user constituency for this
2093+
* function probably doesn't want minus zero, force it to plain zero.
2094+
*/
2095+
if (result == 0.0)
2096+
result = 0.0;
2097+
20742098
CHECKFLOATVAL(result, true /* cotd(0) == Inf */ , true);
20752099
PG_RETURN_FLOAT8(result);
20762100
}
@@ -2133,8 +2157,9 @@ Datum
21332157
dtand(PG_FUNCTION_ARGS)
21342158
{
21352159
float8 arg1 = PG_GETARG_FLOAT8(0);
2136-
int sign = 1;
21372160
float8 result;
2161+
int sign = 1;
2162+
static float8 tan45 = 0.0;
21382163

21392164
/*
21402165
* Per the POSIX spec, return NaN if the input is NaN and throw an error
@@ -2174,6 +2199,26 @@ dtand(PG_FUNCTION_ARGS)
21742199

21752200
result = sign * sind_q1(arg1) / cosd_q1(arg1);
21762201

2202+
/*
2203+
* We want tand(45) to be exactly 1, but the above computation might've
2204+
* produced something different, so scale to get the right result. To
2205+
* avoid redoing sind_q1(45) / cosd_q1(45) many times, and to prevent the
2206+
* compiler from maybe rearranging the calculation, cache that value in a
2207+
* static variable.
2208+
*/
2209+
if (tan45 == 0.0)
2210+
tan45 = sind_q1(45.0) / cosd_q1(45.0);
2211+
2212+
result /= tan45;
2213+
2214+
/*
2215+
* On some machines, we get tand(180) = minus zero, but this isn't always
2216+
* true. For portability, and because the user constituency for this
2217+
* function probably doesn't want minus zero, force it to plain zero.
2218+
*/
2219+
if (result == 0.0)
2220+
result = 0.0;
2221+
21772222
CHECKFLOATVAL(result, true /* tand(90) == Inf */ , true);
21782223
PG_RETURN_FLOAT8(result);
21792224
}
@@ -2188,7 +2233,7 @@ degrees(PG_FUNCTION_ARGS)
21882233
float8 arg1 = PG_GETARG_FLOAT8(0);
21892234
float8 result;
21902235

2191-
result = arg1 * (180.0 / M_PI);
2236+
result = arg1 / RADIANS_PER_DEGREE;
21922237

21932238
CHECKFLOATVAL(result, isinf(arg1), arg1 == 0);
21942239
PG_RETURN_FLOAT8(result);
@@ -2214,7 +2259,7 @@ radians(PG_FUNCTION_ARGS)
22142259
float8 arg1 = PG_GETARG_FLOAT8(0);
22152260
float8 result;
22162261

2217-
result = arg1 * (M_PI / 180.0);
2262+
result = arg1 * RADIANS_PER_DEGREE;
22182263

22192264
CHECKFLOATVAL(result, isinf(arg1), arg1 == 0);
22202265
PG_RETURN_FLOAT8(result);

‎src/test/regress/expected/float8-exp-three-digits-win32.out‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -467,13 +467,13 @@ FROM generate_series(0, 360, 15) AS t(x);
467467
135 | | | -1 | -1
468468
150 | 0.5 | | |
469469
165 | | | |
470-
180 | 0 | -1 | -0 | -Infinity
470+
180 | 0 | -1 | 0 | -Infinity
471471
195 | | | |
472472
210 | -0.5 | | |
473473
225 | | | 1 | 1
474474
240 | | -0.5 | |
475475
255 | | | |
476-
270 | -1 | 0 | -Infinity | -0
476+
270 | -1 | 0 | -Infinity | 0
477477
285 | | | |
478478
300 | | 0.5 | |
479479
315 | | | -1 | -1

‎src/test/regress/expected/float8-small-is-zero.out‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -465,13 +465,13 @@ FROM generate_series(0, 360, 15) AS t(x);
465465
135 | | | -1 | -1
466466
150 | 0.5 | | |
467467
165 | | | |
468-
180 | 0 | -1 | -0 | -Infinity
468+
180 | 0 | -1 | 0 | -Infinity
469469
195 | | | |
470470
210 | -0.5 | | |
471471
225 | | | 1 | 1
472472
240 | | -0.5 | |
473473
255 | | | |
474-
270 | -1 | 0 | -Infinity | -0
474+
270 | -1 | 0 | -Infinity | 0
475475
285 | | | |
476476
300 | | 0.5 | |
477477
315 | | | -1 | -1

‎src/test/regress/expected/float8-small-is-zero_1.out‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -465,13 +465,13 @@ FROM generate_series(0, 360, 15) AS t(x);
465465
135 | | | -1 | -1
466466
150 | 0.5 | | |
467467
165 | | | |
468-
180 | 0 | -1 | -0 | -Infinity
468+
180 | 0 | -1 | 0 | -Infinity
469469
195 | | | |
470470
210 | -0.5 | | |
471471
225 | | | 1 | 1
472472
240 | | -0.5 | |
473473
255 | | | |
474-
270 | -1 | 0 | -Infinity | -0
474+
270 | -1 | 0 | -Infinity | 0
475475
285 | | | |
476476
300 | | 0.5 | |
477477
315 | | | -1 | -1

‎src/test/regress/expected/float8.out‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -467,13 +467,13 @@ FROM generate_series(0, 360, 15) AS t(x);
467467
135 | | | -1 | -1
468468
150 | 0.5 | | |
469469
165 | | | |
470-
180 | 0 | -1 | -0 | -Infinity
470+
180 | 0 | -1 | 0 | -Infinity
471471
195 | | | |
472472
210 | -0.5 | | |
473473
225 | | | 1 | 1
474474
240 | | -0.5 | |
475475
255 | | | |
476-
270 | -1 | 0 | -Infinity | -0
476+
270 | -1 | 0 | -Infinity | 0
477477
285 | | | |
478478
300 | | 0.5 | |
479479
315 | | | -1 | -1

0 commit comments

Comments
 (0)