

Hello hackers, In his blog post (What's new in SQL 2016)[ https://modernsql.com/blog/201706/whatsnewinsql2016], Markus Winand explained some of the changes added to SQL:2016. I spotted that Postgres was behind other RDBMS on hyperbolic functions and log10 function. The log10 function existed but under the name log(<value>). The new functions can be called in a simple select statement :
select log10(100); select sinh(0); select cosh(0); select tanh(0);
Even if Markus Winand had added hyperbolic functions in the paragraph "Trigonometric and Logarithmic Functions", I didn't add hyperbolic function with the trigonometric functions in the documentation, because hyperbolic functions are not trigonometric functions.
I added regression tests for the new functions, but I didn't for log10 function, assuming that if log function worked, log10 will work too.
You'll find enclosed the first version of the patch that can build successfully on my laptop against master. I'm open to any improvement. Cheers,
Lætitia  Think! Do you really need to print this email ? There is no Planet B.


=?UTF8?Q?L=C3=A6titia_Avrot?= < [hidden email]> writes:
> [ adding_log10_and_hyperbolic_functions_v1.patch ]
No objection to the feature, but
 Why are you using the float4width library functions (coshf etc)
rather than the float8width ones (cosh etc)?
 I wonder whether these library functions exist everywhere.
If they don't, what will we do about it ... throw an error?
I see that SUSv2 requires cosh() and so on, so it's quite possible
that these do exist everywhere we care about. I'd be okay with
throwing a patch onto the buildfarm to see, and adding an autoconf
test only if the buildfarm is unhappy. But we should be clear on
what we're going to do about it if that happens.
> I added regression tests for the new functions, but I didn't for log10
> function, assuming that if log function worked, log10 will work too.
Not quite sure I believe that.
Actually, what I'd counsel is that you *not* include tests of what
these do with Inf and NaN. There is no upside to doing so, and lots
of downside if older platforms are squirrely in their behavior, which
is hardly unlikely (cf opossum ...)
regards, tom lane


Hi,
Thanks for your time and advice, Tom! > [ adding_log10_and_hyperbolic_functions_v1.patch ]
No objection to the feature, but
 Why are you using the float4width library functions (coshf etc)
rather than the float8width ones (cosh etc)?
Well, I guess the only reason is that I wasn't paying attention enough... I changed for the float8width library functions.
 I wonder whether these library functions exist everywhere.
If they don't, what will we do about it ... throw an error?
I see that SUSv2 requires cosh() and so on, so it's quite possible
that these do exist everywhere we care about. I'd be okay with
throwing a patch onto the buildfarm to see, and adding an autoconf
test only if the buildfarm is unhappy. But we should be clear on
what we're going to do about it if that happens.
I think I was too confident because of the "it works on my laptop" syndrome... I don't know how to answer to this point.
> I added regression tests for the new functions, but I didn't for log10
> function, assuming that if log function worked, log10 will work too.
Not quite sure I believe that.
Do you mean I should also add a regression test for the new log10 function too ?
Actually, what I'd counsel is that you *not* include tests of what
these do with Inf and NaN. There is no upside to doing so, and lots
of downside if older platforms are squirrely in their behavior, which
is hardly unlikely (cf opossum ...)
I changed the regression tests for hyperbolic functions, so it doesn't test for Inf and NaN.
You'll find enclosed the new version of the patch.
Cheers,
Lætitia


Hi,
Thanks to Emil Iggland's kind review, I added precision on documentation for hyperbolic functions.
I added the patch to the next commitfest.
Cheers,
Lætitia Le dim. 27 janv. 2019 à 20:39, Lætitia Avrot < [hidden email]> a écrit : Hi,
Thanks for your time and advice, Tom! > [ adding_log10_and_hyperbolic_functions_v1.patch ]
No objection to the feature, but
 Why are you using the float4width library functions (coshf etc)
rather than the float8width ones (cosh etc)?
Well, I guess the only reason is that I wasn't paying attention enough... I changed for the float8width library functions.
 I wonder whether these library functions exist everywhere.
If they don't, what will we do about it ... throw an error?
I see that SUSv2 requires cosh() and so on, so it's quite possible
that these do exist everywhere we care about. I'd be okay with
throwing a patch onto the buildfarm to see, and adding an autoconf
test only if the buildfarm is unhappy. But we should be clear on
what we're going to do about it if that happens.
I think I was too confident because of the "it works on my laptop" syndrome... I don't know how to answer to this point.
> I added regression tests for the new functions, but I didn't for log10
> function, assuming that if log function worked, log10 will work too.
Not quite sure I believe that.
Do you mean I should also add a regression test for the new log10 function too ?
Actually, what I'd counsel is that you *not* include tests of what
these do with Inf and NaN. There is no upside to doing so, and lots
of downside if older platforms are squirrely in their behavior, which
is hardly unlikely (cf opossum ...)
I changed the regression tests for hyperbolic functions, so it doesn't test for Inf and NaN.
You'll find enclosed the new version of the patch.
Cheers,
Lætitia
 Think! Do you really need to print this email ? There is no Planet B.


On 2019Jan31, Lætitia Avrot wrote:
> Hi,
>
> Thanks to Emil Iggland's kind review, I added precision on documentation
> for hyperbolic functions.
Hello
I see that in dtanh() you set errno to 0 before calling tanh(), but 1)
you don't check for it afterwards (seems like you should be checking for
ERANGE, as well as checking the return value for isinf()), and 2) you
don't do that in dsinh() and dcosh() and I'm not quite sure I see why.
What's up with that?
Thanks,

Álvaro Herrera https://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Hi Alvaro, Thank you so much for taking the time to review the patch and for taking the time again to sort things out with me this evening.
I see that in dtanh() you set errno to 0 before calling tanh(), but 1)
you don't check for it afterwards (seems like you should be checking for
ERANGE, as well as checking the return value for isinf()), and 2) you
don't do that in dsinh() and dcosh() and I'm not quite sure I see why.
What's up with that?
At the time I wrote that patch, I tried to include errno testing.Then, I think again and came back with the wrong idea everything would be fine.
By rereading math.h documentation, it is now clear that the three functions can raise a ERANGE error.
There are two cases :  range error due to overflow occurs.  range error occurs due to underflow. In that case, the correct result (after rounding) is returned. So I assume we can ignore that case.
For sinh and cosh, we can have both cases and we added support for overflow.
For tanh, the only possible case is underflow and then, the result is correct.
We included comments to explain errno handling in those functions.
Cheers,
Lætitia


>>>>> "Lætitia" == Lætitia Avrot < [hidden email]> writes:
[snip patch]
The spec doesn't require the inverse functions (asinh, acosh, atanh),
but surely there is no principled reason to omit them?

Andrew (irc:RhodiumToad)


Andrew Gierth < [hidden email]> writes:
> The spec doesn't require the inverse functions (asinh, acosh, atanh),
> but surely there is no principled reason to omit them?
+1  AFAICS, the C library has offered all six since C89.
regards, tom lane


Hi Andrew and Tom,
I considered that option before writing my patch but I refrained for 2 reasons:
 There is no consensus about how to name these functions. The standard 80002 goes with arsinh, arcosh and artanh, but you will find easily arcsinh, arccosh and arctanh or even argsinh, argcosh and argtanh. In IT, the names asinh, acosh and atanh are commonly used too. We might implement them with asinh, acosh and atanh names and add aliases if SQL standard decide to add it under other names though.  If we go with inverse hyperbolic functions, I guess we could add other hyperbolic functions as hyperbolic cosecant, secant and cotangent too. Then it adds the inverse hyperbolic functions of these three functions. These six functions are not described in math.h library. I guess it's because these functions are quite simple to deduce from the others.
So, as you're asking that too, maybe my reasons weren't good enough. You'll find enclosed a new version of the patch with asinh, acosh and atanh (v5).
Then I tried for several days to implement the 6 last hyperbolic functions, but I wasn't satisfied with the result, so I just dropped it. Cheers, Lætitia Andrew Gierth <[hidden email]> writes:
> The spec doesn't require the inverse functions (asinh, acosh, atanh),
> but surely there is no principled reason to omit them?
+1  AFAICS, the C library has offered all six since C89.
regards, tom lane
 Think! Do you really need to print this email ? There is no Planet B.


On 12/02/2019 06:44, Lætitia Avrot wrote:
> Hi Andrew and Tom,
>
> I considered that option before writing my patch but I refrained for 2
> reasons:
>
>  There is no consensus about how to name these functions. The
> standard 80002 goes with arsinh, arcosh and artanh,
> but you will find easily arcsinh, arccosh and arctanh or even
> argsinh, argcosh and argtanh. In IT, the names asinh,
> acosh and atanh are commonly used too. We might implement them with
> asinh, acosh and atanh names and add
> aliases if SQL standard decide to add it under other names though.
[...]
>
> Le dim. 3 févr. 2019 à 16:12, Tom Lane < [hidden email]
> <mailto: [hidden email]>> a écrit :
>
> Andrew Gierth < [hidden email]
> <mailto: [hidden email]>> writes:
> > The spec doesn't require the inverse functions (asinh, acosh,
> atanh),
> > but surely there is no principled reason to omit them?
>
> +1  AFAICS, the C library has offered all six since C89.
>
> regards, tom lane
>
[...]
I can only remember coming across the asinh, acosh, and atanh forms. In
45 years of programming.
Cheers,
Gavin


Gavin Flower < [hidden email]> writes:
> On 12/02/2019 06:44, Lætitia Avrot wrote:
>> I considered that option before writing my patch but I refrained for 2
>> reasons:
>>
>>  There is no consensus about how to name these functions. The
>> standard 80002 goes with arsinh, arcosh and artanh,
>> but you will find easily arcsinh, arccosh and arctanh or even
>> argsinh, argcosh and argtanh. In IT, the names asinh,
>> acosh and atanh are commonly used too. We might implement them with
>> asinh, acosh and atanh names and add
>> aliases if SQL standard decide to add it under other names though.
> I can only remember coming across the asinh, acosh, and atanh forms. In
> 45 years of programming.
I don't think this is a problem. Postgres has never had any hesitation
about adopting Cstandard function names if there's nothing in the SQL
standard. The C standard says asinh etc, so those are the names to use.
As Lætitia says, there'd be little problem with adding aliases if someday
the SQL committee decides to add these with other spellings.
regards, tom lane

