Re: [HACKERS] compress method for spgist - 2 - Mailing list 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@komzpa.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?

--