On 08/03/2018 02:39 PM, Tomas Vondra wrote:
On 08/03/2018 06:40 AM, Kyotaro HORIGUCHI wrote:
...

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?


I'm not sure I follow. The patch does not modify non_negative() at all, and we still call it like this:

     if (non_negative(overlap) < non_negative(context->overlap) ||
         (range > context->range &&
          non_negative(overlap) <= non_negative(context->overlap)))
         selectthis = true;

where all the "overlap" values are still float4. The only thing that changed here is that instead of doing the arithmetic operations directly we call float8_mi/float8_div to benefit from the float8 handling.

So I'm not sure how does the patch beaks this? And what do you mean by 'eliminate float4'?


Kyotaro-san, can you explain what your concerns regarding this bit are? I'd like to get 0002 committed, but I'm not sure I understand your point about the changes in gist code, so I can't address them. And I certainly don't want to just ignore them ...

regards

--
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Reply via email to