Horton

Re: [PATCH] Improve geometric types

Tomas Vondra <tomas.vondra@2ndquadrant.com>

View thread View on postgr.es
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Cc: emre@hasegeli.com, tgl@sss.pgh.pa.us, thomas.munro@enterprisedb.com, pgsql-hackers@lists.postgresql.org
Date: 2018-08-08T12:39:54Z
Lists: pgsql-hackers 

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

Commits

  1. Improve test coverage of geometric types

  2. Fix problems in handling the line data type

  3. Use the built-in float datatypes to implement geometric types

  4. Remove remaining GEODEBUG references from geo_ops.c

  5. Provide separate header file for built-in float types

  6. Refactor geometric functions and operators

  7. Fix crash in close_ps() for NaN input coordinates.

  8. Fix GiST index build for NaN values in geometric types.

  9. Enable building with Visual Studion 2013.

  10. Suppress -0 in the C field of lines computed by line_construct_pts().