Re: [Patch] Log10 and hyperbolic functions for SQL:2016 compliance

From: Lætitia Avrot <laetitia(dot)avrot(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [Patch] Log10 and hyperbolic functions for SQL:2016 compliance
Date: 2019-01-31 20:45:25
Message-ID: CAB_COdi8WGgdW80rNC+1R+xnWT3ssfDAe5UT9-95QUNuURSvyQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

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 <laetitia(dot)avrot(at)gmail(dot)com> 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 float4-width library functions (coshf etc)
>> rather than the float8-width ones (cosh etc)?
>>
>> Well, I guess the only reason is that I wasn't paying attention enough...
> I changed for the float8-width 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.*

Attachment Content-Type Size
adding_log10_and_hyperbolic_functions_v3.patch text/x-patch 7.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stas Kelvich 2019-01-31 20:51:36 XLogInsert() of dangling pointer while logging replica identity
Previous Message Tom Lane 2019-01-31 20:28:57 Re: Proposed refactoring of planner header files