Re: compress method for spgist - 2

On 20.09.2017 23:19, Alexander Korotkov wrote:

> On Wed, Sep 20, 2017 at 11:07 PM, Tom Lane <tgl@sss.pgh.pa.us 
> <mailto:tgl@sss.pgh.pa.us>> wrote:
>
>     Darafei Praliaskouski <me@komzpa.net <mailto:me@komzpa.net>> writes:
>     > I have some questions about the circles example though.
>
>     >  * What is the reason for isnan check and swap of box ordinates
>     for circle? It wasn't in the code previously.
>
>     I hadn't paid any attention to this patch previously, but this comment
>     excited my curiosity, so I went and looked:
>
>     +       bbox->high.x = circle->center.x + circle->radius;
>     +       bbox->low.x = circle->center.x - circle->radius;
>     +       bbox->high.y = circle->center.y + circle->radius;
>     +       bbox->low.y = circle->center.y - circle->radius;
>     +
>     +       if (isnan(bbox->low.x))
>     +       {
>     +               double tmp = bbox->low.x;
>     +               bbox->low.x = bbox->high.x;
>     +               bbox->high.x = tmp;
>     +       }
>
>     Maybe I'm missing something, but it appears to me that it's
>     impossible for
>     bbox->low.x to be NaN unless circle->center.x and/or
>     circle->radius is a
>     NaN, in which case bbox->high.x would also have been computed as a
>     NaN,
>     making the swap entirely useless.  Likewise for the Y case.  There
>     may be
>     something useful to do about NaNs here, but this doesn't seem like it.
>
> Yeah, +1.
>

It is possible for bbox->low.x to be NaN when circle->center.x is and
circle->radius are both +Infinity.  Without this float-order-preserving 
swapping
one regression test for KNN with ORDER BY index will be totally broken 
(you can
try it: https://github.com/glukhovn/postgres/tree/knn). Unfortunately, I 
do not
remember exactly why, but most likely because of the incorrect index 
structure.

-- 
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company