On Fri, Nov 27, 2015 at 6:33 PM, Dean Rasheed <dean.a.rash...@gmail.com> wrote: > > On 11 November 2015 at 11:45, Dean Rasheed <dean.a.rash...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers