Re: Proposal: Trigonometric functions in degrees - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Proposal: Trigonometric functions in degrees
Date
Msg-id CAB7nPqQdichAyq3iNo_Ym9jgo6XkyuhivTV5tCG3E9Z_drCo6A@mail.gmail.com
Whole thread Raw
In response to Re: Proposal: Trigonometric functions in degrees  (Dean Rasheed <dean.a.rasheed@gmail.com>)
Responses Re: Proposal: Trigonometric functions in degrees  (Michael Paquier <michael.paquier@gmail.com>)
List pgsql-hackers
On Fri, Nov 27, 2015 at 6:33 PM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>
> On 11 November 2015 at 11:45, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> > Thanks for testing. I'll post an updated patch sometime soon.
> >
>
> I finally got round to looking at this again, and here is an updated patch.

Cool, thanks!

> I've reverted the changes to the CHECKFLOATVAL() checks in the
> existing functions, so that can be a separate discussion. As for the
> checks in the new functions added by this patch, I've left them as
> they were on the grounds that the checks are taking place after
> argument reduction, so the arguments will not be infinite  at that
> point (and besides, I think the checks are correct as written anyway).

On this one, I agree less for the new node but I am fine to let the
committer take the final shot. Making things similar to the existing
functions seems the best approach to me though.

> I've reduced the regression tests down to checks of the cases where
> the results should be exact, so they should now be
> platform-independent. Many of the original tests were useful during
> development to ensure that sane-looking answers were being returned,
> but they didn't really add anything as regression tests, other than
> extra complication due to platform variations.

That's definitely a better thing to do. I got surprised as well for
example by how things behave differently on OSX, Linux and Windows
when testing your patch :)

+        * Stitch together inverse sine and cosine functions for the ranges
+        * [0, 0.5] and [0.5, 1].  Each expression below is guaranteed to return
+        * exactly 30 for x=0.5, so the result is a continuous
monotonic function
+        * over the full range.

+        * Stitch together inverse sine and cosine functions for the ranges
+        * [0, 0.5] and [0.5, 1].  Each expression below is guaranteed to return
+        * exactly 60 for x=0.5, so the result is a continuous
monotonic function
+        * over the full range.

Those two should mention [0,0.5] and (0.5,1]. 0.5 is only part of the
first portion. There are a couple of places where that's not exact as
well, but no real big deal.

Now on OSX the following things are inconsistent:
1) acos:
=# select acosd(1.1);
ERROR:  22003: input is out of range
LOCATION:  dacosd, float.c:1752
Time: 0.623 ms
=# select acos(1.1);acos
------ NaN
(1 row)
=# select asind('Infinity'::float8);
ERROR:  22003: input is out of range
LOCATION:  dasind, float.c:1778
2) asin:
=# select asind(1.1);
ERROR:  22003: input is out of range
LOCATION:  dasind, float.c:1778
=# select asin(1.1);asin
------ NaN
(1 row)
Instinctively, it seems to me that we had better return Nan for the
new asind and acosd when being out of range for OSX, Linux will
complain about an out-of-range error so the code is right in this
case.

3) those ones as well are interesting:
=# select tand(180);tand
------  -0
(1 row)
=# select cotd(-90);cotd
------  -0

Regards,
-- 
Michael



pgsql-hackers by date:

Previous
From: Rahila Syed
Date:
Subject: Re: [PROPOSAL] VACUUM Progress Checker.
Next
From: Michael Paquier
Date:
Subject: Re: Proposal: Trigonometric functions in degrees