Re: [PATCH] Improve geometric types - Mailing list pgsql-hackers

From Tomas Vondra
Subject Re: [PATCH] Improve geometric types
Date
Msg-id d760e38e-e015-6393-fe80-f597f0b5d791@2ndquadrant.com
Whole thread Raw
In response to Re: [PATCH] Improve geometric types  (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>)
Responses Re: [PATCH] Improve geometric types
List pgsql-hackers
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'?


thank you

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


pgsql-hackers by date:

Previous
From: Jesper Pedersen
Date:
Subject: Re: partition tree inspection functions
Next
From: Alexander Kukushkin
Date:
Subject: Re: Standby trying "restore_command" before local WAL