From: Tom Lane <tgl@sss.pgh.pa.us>
To: Darafei Praliaskouski <me@komzpa.net>
cc: pgsql-hackers@postgresql.org, Heikki Linnakangas <hlinnaka@iki.fi>,
        Fedor Sigaev <teodor@sigaev.ru>,
        Alexander Korotkov <aekorotkov@gmail.com>,
        Nikita Glukhov <n.gluhov@postgrespro.ru>
Subject: Re: compress method for spgist - 2
In-reply-to: <20170920190059.1354.29457.pgcf@coridan.postgresql.org>
References: <54907069.1030506@sigaev.ru>
 <CAPpHfdsoy2bJMSTtvQySJ9zN2YJmzWJBLvdHV=ZvMOm6BR0UFg@mail.gmail.com>
 <20170920190059.1354.29457.pgcf@coridan.postgresql.org>
Comments: In-reply-to Darafei Praliaskouski <me@komzpa.net>
	message dated "Wed, 20 Sep 2017 19:00:59 -0000"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <31998.1505938053.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Wed, 20 Sep 2017 16:07:33 -0400
Message-ID: <31999.1505938053@sss.pgh.pa.us>
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

Darafei Praliaskouski <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 circl=
e? 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 =3D circle->center.x + circle->radius;
+ 	bbox->low.x =3D circle->center.x - circle->radius;
+ 	bbox->high.y =3D circle->center.y + circle->radius;
+ 	bbox->low.y =3D circle->center.y - circle->radius;
+=20
+ 	if (isnan(bbox->low.x))
+ 	{
+ 		double tmp =3D bbox->low.x;
+ 		bbox->low.x =3D bbox->high.x;
+ 		bbox->high.x =3D 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.

> How about removing circle from the scope of this patch, so it is smaller =
and cleaner?

Neither of those patches would be particularly large, and since they'd need
to touch adjacent code in some places, the diffs wouldn't be independent.
I think splitting them is just make-work.

			regards, tom lane


--=20
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers