Re: compress method for spgist - 2

From: Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>
To: Alexander Korotkov <aekorotkov(at)gmail(dot)com>, Darafei Komяpa Praliaskouski <me(at)komzpa(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, 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-22 00:03:50
Message-ID: 1499c9d0-075a-3014-d2aa-ba59121b3728@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On 21.09.2017 02:27, Alexander Korotkov wrote:

> On Thu, Sep 21, 2017 at 2:06 AM, Darafei "Komяpa" Praliaskouski
> <me(at)komzpa(dot)net <mailto:me(at)komzpa(dot)net>> wrote:
>
> It is possible for bbox->low.x to be NaN when circle->center.x
> is and
> circle->radius are both +Infinity.
>
>
> What is rationale behind this circle?
>
>
> I would prefer to rather forbid any geometries with infs and nans. 
> However, then upgrade process will suffer.  User with such geometries
> would get errors during dump/restore, pg_upgraded instances would
> still contain invalid values...
>
> It seems to me that any circle with radius of any Infinity should
> become a [-Infinity .. Infinity, -Infinity .. Infinity] box.Then
> you won't have NaNs, and index structure shouldn't be broken.
>
>
> We probably should produce [-Infinity .. Infinity, -Infinity ..
> Infinity] box for any geometry containing inf or nan.  That MBR would
> be founded for any query, saying: "index can't help you for this kind
> value, only recheck can deal with that".  Therefore, we would at least
> guarantee that results of sequential scan and index scan are the same.
>

I have looked at the GiST KNN code and found the same errors for NaNs,
infinities and NULLs as in my SP-GiST KNN patch.

Attached two patches:

1. Fix NaN-inconsistencies in circle's bounding boxes computed in GiST
compress
method leading to the failed Assert(box->low.x <= box->high.x) in
computeDistance() from src/backend/access/gist/gistproc.c by
transforming NaNs
into infinities (corresponding test case provided in the second patch).

2. Fix ordering of NULL, NaN and infinite distances by GiST.  This distance
values could be mixed because NULL distances were transformed into
infinities,
and there was no special processing for NaNs in KNN queue's comparison
function.
At first I tried just to set recheck flag for NULL distances, but it did not
work for index-only scans because they do not support rechecking. So I had
to add a special flag for NULL distances.

Should I start a separate thread for this issue and add patches to
commitfest?

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

Attachment Content-Type Size
0001-Fix-circle-bounding-box-inconsistency-in-GiST-compress-method-v01.patch text/x-patch 1.1 KB
0002-Fix-GiST-ordering-by-distance-for-NULLs-and-NaNs-v01.patch text/x-patch 12.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Haribabu Kommi 2017-09-22 00:16:24 Re: visual studio 2017 build support
Previous Message Michael Paquier 2017-09-21 23:02:35 Re: Assertion failure when the non-exclusive pg_stop_backup aborted.