From: | Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | tomas(dot)vondra(at)2ndquadrant(dot)com |
Cc: | emre(at)hasegeli(dot)com, tgl(at)sss(dot)pgh(dot)pa(dot)us, thomas(dot)munro(at)enterprisedb(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: [PATCH] Improve geometric types |
Date: | 2018-08-03 04:40:59 |
Message-ID: | 20180803.134059.182543919.horiguchi.kyotaro@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
At Thu, 2 Aug 2018 11:50:55 +0200, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> wrote in <ce3cf95a-4751-c168-54ae-636c486e06cd(at)2ndquadrant(dot)com>
>
>
> 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.
I'm not confident on replacing double to float8 partially in gist
code. After the 0002 patch applied, I see most of problematic
usage of double or bare arithmetic on dimentional values in
gistproc.c.
> static inline float
> non_negative(float val)
> {
> if (val >= 0.0f)
> return val;
> else
> return 0.0f;
> }
It is used as "non_negative(overlap)", where overlap is float4,
which is calculated using float8_mi. Float4 makes sense only if
we need to store a large number of it to somewhere but they are
just working varialbles. Couldn't we eliminate float4 that
doesn't have a requirement to do so?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
From | Date | Subject | |
---|---|---|---|
Next Message | Kyotaro HORIGUCHI | 2018-08-03 04:57:29 | Re: [PATCH] Improve geometric types |
Previous Message | Kyotaro HORIGUCHI | 2018-08-03 04:13:11 | Re: Problem while updating a foreign table pointing to a partitioned table on foreign server |