pgsql: Add trigonometric functions that work in degrees.

On 01/22/2016 03:46 PM, Tom Lane wrote:

Add trigonometric functions that work in degrees.

The implementations go to some lengths to deliver exact results for values
where an exact result can be expected, such as sind(30) = 0.5 exactly.

I have a host here that is having regression test failures from this commit:

--- src/test/regress/expected/float8.out
+++ src/test/regress/results/float8.out
@@ -490,9 +490,9 @@
    x   | asind | acosd | atand
  ------+-------+-------+-------
     -1 |   -90 |   180 |   -45
- -0.5 |   -30 |   120 |
+ -0.5 |       |   120 |
      0 |     0 |    90 |     0
-  0.5 |    30 |    60 |
+  0.5 |       |    60 |
      1 |    90 |     0 |    45
  (5 rows)

Any ideas?

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Apr 5, 2016 at 10:16 AM, Peter Eisentraut <peter_e@gmx.net> wrote:

On 01/22/2016 03:46 PM, Tom Lane wrote:

Add trigonometric functions that work in degrees.

The implementations go to some lengths to deliver exact results for values
where an exact result can be expected, such as sind(30) = 0.5 exactly.

I have a host here that is having regression test failures from this commit:

--- src/test/regress/expected/float8.out
+++ src/test/regress/results/float8.out
@@ -490,9 +490,9 @@
x   | asind | acosd | atand
------+-------+-------+-------
-1 |   -90 |   180 |   -45
- -0.5 |   -30 |   120 |
+ -0.5 |       |   120 |
0 |     0 |    90 |     0
-  0.5 |    30 |    60 |
+  0.5 |       |    60 |
1 |    90 |     0 |    45
(5 rows)

Any ideas?

Likely an oversight not tracked by the buildfarm. What are you using here?
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Michael Paquier <michael.paquier@gmail.com> writes:

On Tue, Apr 5, 2016 at 10:16 AM, Peter Eisentraut <peter_e@gmx.net> wrote:

On 01/22/2016 03:46 PM, Tom Lane wrote:

Add trigonometric functions that work in degrees.

I have a host here that is having regression test failures from this commit:

Likely an oversight not tracked by the buildfarm. What are you using here?

Indeed. Also, I trust you're checking 00347575e or later, and not
e1bd684a3 itself?

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 04/04/2016 09:20 PM, Michael Paquier wrote:

Likely an oversight not tracked by the buildfarm. What are you using here?

It should be a rather unspectactular Debian system. I have tried a
bunch of different compilers and optimization options, without success.
I'll keep looking.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Peter Eisentraut <peter_e@gmx.net> writes:

On 04/04/2016 09:20 PM, Michael Paquier wrote:

Likely an oversight not tracked by the buildfarm. What are you using here?

It should be a rather unspectactular Debian system. I have tried a
bunch of different compilers and optimization options, without success.
I'll keep looking.

Perhaps it'd be worthwhile to check the behavior before and after each
of the portability patches that followed on to e1bd684a3, and see if any
of them change the behavior for you (and if so just how). That might
give us a clue what's going on.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Peter Eisentraut <peter_e@gmx.net> writes:

On 01/22/2016 03:46 PM, Tom Lane wrote:

Add trigonometric functions that work in degrees.

I have a host here that is having regression test failures from this commit:

--- src/test/regress/expected/float8.out
+++ src/test/regress/results/float8.out
@@ -490,9 +490,9 @@
x   | asind | acosd | atand
------+-------+-------+-------
-1 |   -90 |   180 |   -45
- -0.5 |   -30 |   120 |
+ -0.5 |       |   120 |
0 |     0 |    90 |     0
-  0.5 |    30 |    60 |
+  0.5 |       |    60 |
1 |    90 |     0 |    45
(5 rows)

BTW ... looking closer at that, it appears to show asind(-0.5) and
asind(0.5) returning NULL. Which makes no sense at all, because
there is no provision in dasind() for returning a null, regardless
of the input value.

So I'm pretty baffled. Maybe you could step through this and figure
out where it's going off the rails?

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Apr 8, 2016 at 5:48 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

On 01/22/2016 03:46 PM, Tom Lane wrote:

Add trigonometric functions that work in degrees.

I have a host here that is having regression test failures from this commit:

--- src/test/regress/expected/float8.out
+++ src/test/regress/results/float8.out
@@ -490,9 +490,9 @@
x   | asind | acosd | atand
------+-------+-------+-------
-1 |   -90 |   180 |   -45
- -0.5 |   -30 |   120 |
+ -0.5 |       |   120 |
0 |     0 |    90 |     0
-  0.5 |    30 |    60 |
+  0.5 |       |    60 |
1 |    90 |     0 |    45
(5 rows)

BTW ... looking closer at that, it appears to show asind(-0.5) and
asind(0.5) returning NULL. Which makes no sense at all, because
there is no provision in dasind() for returning a null, regardless
of the input value.

So I'm pretty baffled. Maybe you could step through this and figure
out where it's going off the rails?

Peter, are you going to look into this further? This is on the open
items list, but there seems to be nothing that can be done about it by
anyone other than, maybe, you.

If you're not going to look into it, I think we should delete the open
item. There's no point in tracking issues that aren't actionable.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 04/15/2016 02:04 PM, Robert Haas wrote:

Peter, are you going to look into this further? This is on the open
items list, but there seems to be nothing that can be done about it by
anyone other than, maybe, you.

I don't know if it's worth tracking this as an open item and thus
kind-of release blocker if no one else has the problem. But I
definitely still have it. My initial suspicion was that this had
something to do with a partial upgrade to gcc 6 (not yet released), in
other words a messed up system. But I was able to reproduce it in a
freshly installed chroot. It only happens with various versions of gcc,
but not with clang. So I'm going to have to keep digging.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Apr 18, 2016 at 12:31 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

I don't know if it's worth tracking this as an open item and thus
kind-of release blocker if no one else has the problem. But I
definitely still have it. My initial suspicion was that this had
something to do with a partial upgrade to gcc 6 (not yet released), in
other words a messed up system. But I was able to reproduce it in a
freshly installed chroot. It only happens with various versions of gcc,
but not with clang. So I'm going to have to keep digging.

gcc is moving slowly but surely to have 6.0 in a released state btw:
https://gcc.gnu.org/ml/gcc/2016-04/msg00103.html
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Michael Paquier <michael.paquier@gmail.com> writes:

On Mon, Apr 18, 2016 at 12:31 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

I don't know if it's worth tracking this as an open item and thus
kind-of release blocker if no one else has the problem. But I
definitely still have it. My initial suspicion was that this had
something to do with a partial upgrade to gcc 6 (not yet released), in
other words a messed up system. But I was able to reproduce it in a
freshly installed chroot. It only happens with various versions of gcc,
but not with clang. So I'm going to have to keep digging.

gcc is moving slowly but surely to have 6.0 in a released state btw:
https://gcc.gnu.org/ml/gcc/2016-04/msg00103.html

Given that it's apparently showing the results of asind as NULL, the
theory that comes to mind is some sort of optimization issue affecting
the output tuple's null-flags. I have no idea why only this test would
be affected, though. Anyway, a good way to test that theory would be
to see if the -O level affects it.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Apr 18, 2016 at 09:17:46AM -0400, Tom Lane wrote:

Michael Paquier <michael.paquier@gmail.com> writes:

On Mon, Apr 18, 2016 at 12:31 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

I don't know if it's worth tracking this as an open item and thus
kind-of release blocker if no one else has the problem. But I
definitely still have it. My initial suspicion was that this had
something to do with a partial upgrade to gcc 6 (not yet released), in
other words a messed up system. But I was able to reproduce it in a
freshly installed chroot. It only happens with various versions of gcc,
but not with clang. So I'm going to have to keep digging.

gcc is moving slowly but surely to have 6.0 in a released state btw:
https://gcc.gnu.org/ml/gcc/2016-04/msg00103.html

Given that it's apparently showing the results of asind as NULL, the
theory that comes to mind is some sort of optimization issue affecting
the output tuple's null-flags. I have no idea why only this test would
be affected, though. Anyway, a good way to test that theory would be
to see if the -O level affects it.

I doubt asind is returning NULL. Here's the query, which uses a CASE to
report NULL if asind returns any value not on a whitelist:

SELECT x,
CASE WHEN asind(x) IN (-90,-30,0,30,90) THEN asind(x) END AS asind,
CASE WHEN acosd(x) IN (0,60,90,120,180) THEN acosd(x) END AS acosd,
CASE WHEN atand(x) IN (-45,0,45) THEN atand(x) END AS atand
FROM (VALUES (-1), (-0.5), (0), (0.5), (1)) AS t(x);

I can see the benefit for atand(-0.5) and for atand(0.5), since those are
inexact. Does the CASE gain us anything for asind or acosd?

Results under -O0 would be a helpful data point, nonetheless.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Noah Misch <noah@leadboat.com> writes:

On Mon, Apr 18, 2016 at 09:17:46AM -0400, Tom Lane wrote:

Given that it's apparently showing the results of asind as NULL ...

I doubt asind is returning NULL. Here's the query, which uses a CASE to
report NULL if asind returns any value not on a whitelist:

SELECT x,
CASE WHEN asind(x) IN (-90,-30,0,30,90) THEN asind(x) END AS asind,
CASE WHEN acosd(x) IN (0,60,90,120,180) THEN acosd(x) END AS acosd,
CASE WHEN atand(x) IN (-45,0,45) THEN atand(x) END AS atand
FROM (VALUES (-1), (-0.5), (0), (0.5), (1)) AS t(x);

Oh, duh --- should have checked the query. Yes, the most probable theory
must be that it's returning something that's slightly off from the exact
value.

I can see the benefit for atand(-0.5) and for atand(0.5), since those are
inexact. Does the CASE gain us anything for asind or acosd?

None of these are expected to be inexact. The point of the CASE is to
make it obvious if what's returned isn't *exactly* what we expect.

We could alternatively set extra_float_digits to its max value and hope
that off-by-one-in-the-last-place values would get printed as something
visibly different from the exact result. I'm not sure I want to trust
that that works reliably; but maybe it would be worth printing the
result both ways, just to provide additional info when there's a failure.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Apr 18, 2016 at 10:22:59PM -0400, Tom Lane wrote:

Noah Misch <noah@leadboat.com> writes:

On Mon, Apr 18, 2016 at 09:17:46AM -0400, Tom Lane wrote:

Given that it's apparently showing the results of asind as NULL ...

I doubt asind is returning NULL. Here's the query, which uses a CASE to
report NULL if asind returns any value not on a whitelist:

SELECT x,
CASE WHEN asind(x) IN (-90,-30,0,30,90) THEN asind(x) END AS asind,
CASE WHEN acosd(x) IN (0,60,90,120,180) THEN acosd(x) END AS acosd,
CASE WHEN atand(x) IN (-45,0,45) THEN atand(x) END AS atand
FROM (VALUES (-1), (-0.5), (0), (0.5), (1)) AS t(x);

Oh, duh --- should have checked the query. Yes, the most probable theory
must be that it's returning something that's slightly off from the exact
value.

I can see the benefit for atand(-0.5) and for atand(0.5), since those are
inexact. Does the CASE gain us anything for asind or acosd?

None of these are expected to be inexact. The point of the CASE is to
make it obvious if what's returned isn't *exactly* what we expect.

Ah, got it.

We could alternatively set extra_float_digits to its max value and hope
that off-by-one-in-the-last-place values would get printed as something
visibly different from the exact result. I'm not sure I want to trust
that that works reliably; but maybe it would be worth printing the
result both ways, just to provide additional info when there's a failure.

We'd have an independent problem if extra_float_digits=3 prints the same
digits for distinguishable float values, so I wouldn't mind relying on it not
to do that. But can we expect the extra_float_digits=3 representation of
those particular values to be the same for every implementation?

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Noah Misch <noah@leadboat.com> writes:

On Mon, Apr 18, 2016 at 10:22:59PM -0400, Tom Lane wrote:

We could alternatively set extra_float_digits to its max value and hope
that off-by-one-in-the-last-place values would get printed as something
visibly different from the exact result. I'm not sure I want to trust
that that works reliably; but maybe it would be worth printing the
result both ways, just to provide additional info when there's a failure.

We'd have an independent problem if extra_float_digits=3 prints the same
digits for distinguishable float values, so I wouldn't mind relying on it not
to do that. But can we expect the extra_float_digits=3 representation of
those particular values to be the same for every implementation?

Hm? The expected answer is exact (30, 45, or whatever) in each case.
If we get some residual low-order digits then it's a failure, so we don't
need to worry about whether it's the same failure everywhere.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Apr 18, 2016 at 11:56:55PM -0400, Tom Lane wrote:

Noah Misch <noah@leadboat.com> writes:

On Mon, Apr 18, 2016 at 10:22:59PM -0400, Tom Lane wrote:

We could alternatively set extra_float_digits to its max value and hope
that off-by-one-in-the-last-place values would get printed as something
visibly different from the exact result. I'm not sure I want to trust
that that works reliably; but maybe it would be worth printing the
result both ways, just to provide additional info when there's a failure.

We'd have an independent problem if extra_float_digits=3 prints the same
digits for distinguishable float values, so I wouldn't mind relying on it not
to do that. But can we expect the extra_float_digits=3 representation of
those particular values to be the same for every implementation?

Hm? The expected answer is exact (30, 45, or whatever) in each case.
If we get some residual low-order digits then it's a failure, so we don't
need to worry about whether it's the same failure everywhere.

Does something forbid snprintf implementations from printing '45'::float8 as
45.0000000000000001 under extra_float_digits=3?

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 19 April 2016 at 05:16, Noah Misch <noah@leadboat.com> wrote:

On Mon, Apr 18, 2016 at 11:56:55PM -0400, Tom Lane wrote:

Noah Misch <noah@leadboat.com> writes:

On Mon, Apr 18, 2016 at 10:22:59PM -0400, Tom Lane wrote:

We could alternatively set extra_float_digits to its max value and hope
that off-by-one-in-the-last-place values would get printed as something
visibly different from the exact result. I'm not sure I want to trust
that that works reliably; but maybe it would be worth printing the
result both ways, just to provide additional info when there's a failure.

We'd have an independent problem if extra_float_digits=3 prints the same
digits for distinguishable float values, so I wouldn't mind relying on it not
to do that. But can we expect the extra_float_digits=3 representation of
those particular values to be the same for every implementation?

Hm? The expected answer is exact (30, 45, or whatever) in each case.
If we get some residual low-order digits then it's a failure, so we don't
need to worry about whether it's the same failure everywhere.

Does something forbid snprintf implementations from printing '45'::float8 as
45.0000000000000001 under extra_float_digits=3?

I'm not sure it's really worth having the test output something like
45.0000000000000001 since that extra detail doesn't really seem
particularly useful beyond the fact that the result wasn't exactly 45.
Also you'd have to be careful how you modified the test, since it's
possible that 45.0000000000000001 might be printed as '45' even under
extra_float_digits=3 and so there'd be a risk of the test passing when
it ought to fail, unless you also printed something else out to
indicate exactness.

Thinking about the actual failure in this case (asind(0.5) not
returning exactly 30) a couple of theories spring to mind. One is that
the compiler is being more aggressive over function inlining, so
init_degree_constants() is being inlined, and then asin_0_5 is being
evaluated at compile time rather than runtime, giving a slightly
different result, as happened in the original version of this patch.
Another theory is that the compiler is performing an unsafe ordering
rearrangement and transforming (asin(x) / asin_0_5) * 30.0 into
asin(x) * (30.0 / asin_0_5). This might happen for example under
something like -funsafe-math-optimizations.

I did a search for other people encountering similar problems and I
came across the following quite interesting example in the Gnu
Scientific Library's implementation of log1p() --
https://github.com/ampl/gsl/blob/master/sys/log1p.c. The reason the
code is now written in that way is in response to this bug:
https://lists.gnu.org/archive/html/bug-gsl/2007-07/msg00008.html with
an overly aggressive compiler.

So using that technique, we might try using a volatile local variable
to enforce the desired evaluation order, e.g.:

volatile double tmp;

tmp = asin(x) / asin_0_5;
return tmp * 30.0;

A similar trick could be used to protect against
init_degree_constants() being inlined, by writing it as

volatile double one_half = 0.5;

asin_0_5 = asin(one_half);

since then the compiler wouldn't be able to assume that one_half was
still equal to 0.5, and wouldn't be able to optimise away the runtime
evaluation of asin() in favour of a compile-time constant.

These are both somewhat unconventional uses of volatile, but I think
they stand a better chance of being more portable, and also more
future-proof against compilers that might in the future make more
advanced code inlining and rearrangement choices.

Of course this is all pure speculation at this stage, but it seems
like it's worth a try.

Regards,
Dean

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Dean Rasheed <dean.a.rasheed@gmail.com> writes:

On 19 April 2016 at 05:16, Noah Misch <noah@leadboat.com> wrote:

On Mon, Apr 18, 2016 at 11:56:55PM -0400, Tom Lane wrote:

Hm? The expected answer is exact (30, 45, or whatever) in each case.
If we get some residual low-order digits then it's a failure, so we don't
need to worry about whether it's the same failure everywhere.

Does something forbid snprintf implementations from printing '45'::float8 as
45.0000000000000001 under extra_float_digits=3?

I'm not sure it's really worth having the test output something like
45.0000000000000001 since that extra detail doesn't really seem
particularly useful beyond the fact that the result wasn't exactly 45.
Also you'd have to be careful how you modified the test, since it's
possible that 45.0000000000000001 might be printed as '45' even under
extra_float_digits=3 and so there'd be a risk of the test passing when
it ought to fail, unless you also printed something else out to
indicate exactness.

Yeah, what I was thinking of printing is something like

asind(x),
asind(x) IN (-90,-30,0,30,90) AS asind_exact,
...

with extra_float_digits=3. The point of this is not necessarily to give
any extra information, though it might, but for failures to be more easily
interpretable. If I'd forgotten how the test worked just a few months
after committing it, how likely is it that some random user faced with a
similar failure would understand what they were seeing?

Also, though I agree that it might not help much to know whether the
output is 45.0000000000000001 or 44.9999999999999999, our thoughts would
be trending in quite a different direction if it turns out that the
output is radically wrong, or even a NaN. The existing test cannot
exclude that possibility.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 19 April 2016 at 14:38, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Yeah, what I was thinking of printing is something like

asind(x),
asind(x) IN (-90,-30,0,30,90) AS asind_exact,
...

with extra_float_digits=3. The point of this is not necessarily to give
any extra information, though it might, but for failures to be more easily
interpretable. If I'd forgotten how the test worked just a few months
after committing it, how likely is it that some random user faced with a
similar failure would understand what they were seeing?

Also, though I agree that it might not help much to know whether the
output is 45.0000000000000001 or 44.9999999999999999, our thoughts would
be trending in quite a different direction if it turns out that the
output is radically wrong, or even a NaN. The existing test cannot
exclude that possibility.

OK, that sounds like it would be a useful improvement to the tests.

Regards,
Dean

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Dean Rasheed <dean.a.rasheed@gmail.com> writes:

On 19 April 2016 at 14:38, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Yeah, what I was thinking of printing is something like

asind(x),
asind(x) IN (-90,-30,0,30,90) AS asind_exact,
...

with extra_float_digits=3.

OK, that sounds like it would be a useful improvement to the tests.

Pushed. Peter, what results do you get from these tests on your
problematic machine?

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers