Re: Strange behavior with polygon and NaN - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Strange behavior with polygon and NaN
Date
Msg-id 1757392.1605998033@sss.pgh.pa.us
Whole thread Raw
In response to Re: Strange behavior with polygon and NaN  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Strange behavior with polygon and NaN  (Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
List pgsql-hackers
I went ahead and pushed 0001 and 0003 (the latter in two parts), since
they didn't seem particularly controversial to me.  Just to keep the
cfbot from whining, here's a rebased version of 0002.

            regards, tom lane

diff --git a/src/backend/utils/adt/geo_ops.c b/src/backend/utils/adt/geo_ops.c
index c1dc511a1a..d9f577bd8b 100644
--- a/src/backend/utils/adt/geo_ops.c
+++ b/src/backend/utils/adt/geo_ops.c
@@ -2719,9 +2719,14 @@ lseg_interpt_line(Point *result, LSEG *lseg, LINE *line)
  *-------------------------------------------------------------------*/

 /*
- * If *result is not NULL, it is set to the intersection point of a
- * perpendicular of the line through the point.  Returns the distance
- * of those two points.
+ * Determine the closest point on the given line to the given point.
+ * If result is not NULL, *result is set to the coordinates of that point.
+ * In any case, returns the distance from the point to the line.
+ * Returns NaN for any ill-defined value.
+ *
+ * NOTE: in some cases the distance is well defined (i.e., infinity)
+ * even though the specific closest point is not.  Hence, if you care
+ * about whether the point is well-defined, check its coordinates for NaNs.
  */
 static float8
 line_closept_point(Point *result, LINE *line, Point *point)
@@ -2729,17 +2734,30 @@ line_closept_point(Point *result, LINE *line, Point *point)
     Point        closept;
     LINE        tmp;

+    /*
+     * If it is unclear whether the point is on the line or not, then the
+     * results are ill-defined.  This eliminates cases where any of the given
+     * coordinates are NaN, as well as cases where infinite coordinates give
+     * rise to Inf - Inf, 0 * Inf, etc.
+     */
+    if (unlikely(isnan(float8_pl(float8_pl(float8_mul(line->A, point->x),
+                                           float8_mul(line->B, point->y)),
+                                 line->C))))
+    {
+        if (result != NULL)
+            result->x = result->y = get_float8_nan();
+        return get_float8_nan();
+    }
+
     /*
      * We drop a perpendicular to find the intersection point.  Ordinarily we
-     * should always find it, but that can fail in the presence of NaN
-     * coordinates, and perhaps even from simple roundoff issues.
+     * should always find it, but perhaps roundoff issues could prevent that.
      */
     line_construct(&tmp, point, line_invsl(line));
     if (!line_interpt_line(&closept, &tmp, line))
     {
         if (result != NULL)
-            *result = *point;
-
+            result->x = result->y = get_float8_nan();
         return get_float8_nan();
     }

@@ -2758,7 +2776,9 @@ close_pl(PG_FUNCTION_ARGS)

     result = (Point *) palloc(sizeof(Point));

-    if (isnan(line_closept_point(result, line, pt)))
+    (void) line_closept_point(result, line, pt);
+
+    if (unlikely(isnan(result->x) || isnan(result->y)))
         PG_RETURN_NULL();

     PG_RETURN_POINT_P(result);
diff --git a/src/test/regress/expected/geometry.out b/src/test/regress/expected/geometry.out
index c4f853ae9f..6210075bc1 100644
--- a/src/test/regress/expected/geometry.out
+++ b/src/test/regress/expected/geometry.out
@@ -1070,9 +1070,9 @@ SELECT p.f1, l.s, p.f1 ## l.s FROM POINT_TBL p, LINE_TBL l;
  (1e+300,Infinity) | {0,-1,5}                              | (1e+300,5)
  (1e+300,Infinity) | {1,0,5}                               |
  (1e+300,Infinity) | {0,3,0}                               | (1e+300,0)
- (1e+300,Infinity) | {1,-1,0}                              | (Infinity,NaN)
- (1e+300,Infinity) | {-0.4,-1,-6}                          | (-Infinity,NaN)
- (1e+300,Infinity) | {-0.000184615384615,-1,15.3846153846} | (-Infinity,NaN)
+ (1e+300,Infinity) | {1,-1,0}                              |
+ (1e+300,Infinity) | {-0.4,-1,-6}                          |
+ (1e+300,Infinity) | {-0.000184615384615,-1,15.3846153846} |
  (1e+300,Infinity) | {3,NaN,5}                             |
  (1e+300,Infinity) | {NaN,NaN,NaN}                         |
  (1e+300,Infinity) | {0,-1,3}                              | (1e+300,3)

pgsql-hackers by date:

Previous
From: Peter Geoghegan
Date:
Subject: Re: Different results between PostgreSQL and Oracle for "for update" statement
Next
From: Tom Lane
Date:
Subject: Re: [PATCH] Covering SPGiST index