Re: BUG #17334: Assert failed inside computeDistance() on gist index scanning

At Mon, 13 Dec 2021 03:32:18 +0100, Tomas Vondra <tomas.vondra@enterprisedb.com> wrote in 
> On 12/13/21 00:37, Tom Lane wrote:
> > Tomas Vondra <tomas.vondra@enterprisedb.com> writes:
> >> FWIW I can reproduce this on master too. The failure happens because
> >> of
> >> NaN value in the index:
> > Man, what a pain those are.
> > 
> >> I'm not sure if the issue is in allowing the NaN to be added to the
> >> index, or not handling it correctly during the index scan.
> > Surely omitting the entry from the index would lead to incorrect
> > answers.  Without any index, we get
> > regression=# CREATE TABLE point_tbl(f1 point);
> > CREATE TABLE
> > regression=# INSERT INTO point_tbl SELECT ('0,0') FROM
> > generate_series(1, 2);
> > INSERT 0 2
> > regression=# INSERT INTO point_tbl VALUES ('0,NaN');
> > INSERT 0 1
> > regression=# SELECT f1, f1 <-> point '(0,0)' AS dist FROM point_tbl
> > ORDER BY dist;
> >     f1    | dist
> > ---------+------
> >   (0,0)   |    0
> >   (0,0)   |    0
> >   (0,NaN) |  NaN
> > (3 rows)
> > You can argue about where the NaN distance should sort, but
> > not about whether the row should appear at all.

We difined NaN as larger value than any non-NaN and that seems a
not-bad compromise.  Anyway there seems to be no other way than
individually fixing every code path that use float types X( For
example, the attached "fixes" the case of the example only. However,
that is a kind of nerve-wrecking..

> >> It's interesting btree_gist does not have issues (for NaN in float8
> >> columns). It seems not to store NaN in the index,  It seems to replace
> >> them with tiny values, at least according to pageinspect.
> > Yipes, that's even worse, if true. >
> 
> Yeah. I haven't looked at the code, but this is what I see:
> 
>   test=# create extension btree_gist ;
>   CREATE EXTENSION
>   test=# CREATE TABLE t(f1 double precision);
>   CREATE TABLE
>   test=# INSERT INTO t VALUES ('NaN'::float8);
>   INSERT 0 1
>   test=# CREATE INDEX idx ON t USING gist(f1);
>   CREATE INDEX
>   test=# select * from gist_page_items(get_raw_page('idx', 0),
>   'idx'::regclass);
>    itemoffset | ctid  | itemlen | dead |         keys
>   ------------+-------+---------+------+-----------------------
>             1 | (0,1) |      24 | f    | (f1)=(1.9187051e-316)
>   (1 row)
> 
> 
> test=# set enable_seqscan = on;
> SET
> test=# select * from t where f1 = 'NaN'::float8;
>  f1
> -----
>  NaN
> (1 row)
> 
> 
> test=# set enable_seqscan = off;
> SET
> test=# select * from t where f1 = 'NaN'::float8;
>  f1
> ----
> (0 rows)
> 
> 
> So yeah, that seems like an index corruption. Moreover:
> 
> test=# INSERT INTO t VALUES (0::float8);
> test=# INSERT INTO t VALUES ('NaN'::float8);
> test=# INSERT INTO t VALUES (0::float8);
> 
> test=# select * from t order by f1 <-> 0;
>  f1
> -----
>  NaN
>    0
>    0
>  NaN
> (4 rows)

Too bad \^_^/

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/access/gist/gistproc.c b/src/backend/access/gist/gistproc.c
index d474612b77..19a7d5ef2b 100644
--- a/src/backend/access/gist/gistproc.c
+++ b/src/backend/access/gist/gistproc.c
@@ -1237,11 +1237,15 @@ computeDistance(bool isLeaf, BOX *box, Point *point)
     else if (point->x <= box->high.x && point->x >= box->low.x)
     {
         /* point is over or below box */
-        Assert(box->low.y <= box->high.y);
+        Assert(box->low.y <= box->high.y
+               || isnan(box->low.y) || isnan(box->high.y));
+
         if (point->y > box->high.y)
             result = float8_mi(point->y, box->high.y);
         else if (point->y < box->low.y)
             result = float8_mi(box->low.y, point->y);
+        else if (isnan(point->y) || isnan(box->high.y) || isnan(box->low.y))
+            result = get_float8_nan();
         else
             elog(ERROR, "inconsistent point values");
     }