Re: compress method for spgist - 2

From: Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>
To: Alexander Korotkov <aekorotkov(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Darafei Praliaskouski <me(at)komzpa(dot)net>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Fedor Sigaev <teodor(at)sigaev(dot)ru>
Subject: Re: compress method for spgist - 2
Date: 2017-09-20 21:57:49
Message-ID: 60e048d3-aa29-8289-5438-3b0c4c1d8d84@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On 20.09.2017 23:19, Alexander Korotkov wrote:

> On Wed, Sep 20, 2017 at 11:07 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us
> <mailto:tgl(at)sss(dot)pgh(dot)pa(dot)us>> wrote:
>
> Darafei Praliaskouski <me(at)komzpa(dot)net <mailto:me(at)komzpa(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Julien Rouhaud 2017-09-20 22:00:01 Re: [Proposal] Make the optimiser aware of partitions ordering
Previous Message Thomas Munro 2017-09-20 21:50:28 Re: Error: dsa_area could not attach to a segment that has been freed