On 08/01/2018 01:40 PM, Emre Hasegeli wrote:
>> Ah, so there's an assumption that NaNs are handled earlier and never reach
>> this place? That's probably a safe assumption. I haven't thought about that,
>> it simply seemed suspicious that the code mixes direct comparisons and
>> float8_mi() calls.
>
> The comparison functions handle NaNs. The arithmetic functions handle
> returning error on underflow, overflow and division by zero. I
> assumed we want to return error on those in any case, but we don't
> want to handle NaNs at every place.
>
>> Not sure, I'll leave that up to you. I don't mind doing it in a separate
>> patch (I'd probably prefer that over mixing it into unrelated patch).
>
> It is attached separately.
>
OK, thanks.
So, have we reached conclusion about all the bits I mentioned on 7/31?
The delta and float8/double cast are fixed, and for computeDistance
(i.e. doing comparisons directly or using float8_lt), the code may seem
a bit inconsistent, but it is in fact correct as the NaNs are handled
elsewhere. That seems reasonable, but perhaps a comment pointing that
out would be nice.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services