From: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> |
---|---|
To: | Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> |
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-16 18:00:45 |
Message-ID: | 3cb6f529-090b-8416-1091-31708245537a@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 08/09/2018 11:42 AM, Kyotaro HORIGUCHI wrote:
> At Wed, 8 Aug 2018 14:39:54 +0200, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> wrote in <6ecb4f61-1fb1-08a1-31d6-e58e9c352374(at)2ndquadrant(dot)com>
>>
>>
>> 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 ...
>
> It doesn't break nothing so nothing must be done with this. Just
> I was a bit uneasy to see meaninglessly used foat4. Sorry for
> the unnecessary argument.
>
> After all I don't object to commit it in this shape.
>
Understood. Thanks for the explanation.
I've pushed parts 0001 and 0002, as submitted on August 1. Let's see if
that upsets some of the buildfarm animals.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Jesper Pedersen | 2018-08-16 18:23:10 | Re: Index Skip Scan |
Previous Message | Shay Rojansky | 2018-08-16 17:54:25 | Re: Stored procedures and out parameters |