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
