Wrong results from in_range() tests with infinite offset - Mailing list pgsql-hackers

Dean Rasheed pointed out that in_range for float4/float8 seems to be
doing the wrong thing for infinite offsets, and after some testing
I concur that it is.  For example, a sort key of '-infinity' should
be considered to be in-range for a range specified as RANGE BETWEEN
'inf' PRECEDING AND 'inf' PRECEDING; but with the code as it stands,
it isn't.  I propose the attached patch, which probably should be
back-patched.

When the current row's value is +infinity, actual computation of
base - offset would yield NaN, making it a bit unclear whether
we should consider -infinity to be in-range.  It seems to me that
we should, as that gives more natural-looking results in the test
cases, so that's how the patch does it.

            regards, tom lane

diff --git a/src/backend/utils/adt/float.c b/src/backend/utils/adt/float.c
index 50ec3d3dde..5c722d85c3 100644
--- a/src/backend/utils/adt/float.c
+++ b/src/backend/utils/adt/float.c
@@ -1090,11 +1090,18 @@ in_range_float8_float8(PG_FUNCTION_ARGS)
     /*
      * Deal with infinite offset (necessarily +inf, at this point).  We must
      * special-case this because if base happens to be -inf, their sum would
-     * be NaN, which is an overflow-ish condition we should avoid.
+     * be NaN, which is an overflow-ish condition we must avoid.  We take that
+     * sum to be +inf; that is, we consider base + inf to be +inf for any
+     * non-NaN base.  Conversely, base - inf is taken to be -inf even if base
+     * is +inf.  These choices may seem arbitrary, but they give desirable
+     * windowing behavior with an infinite offset.
      */
     if (isinf(offset))
     {
-        PG_RETURN_BOOL(sub ? !less : less);
+        if (sub)
+            sum = -get_float8_infinity();
+        else
+            sum = get_float8_infinity();
     }

     /*
@@ -1103,7 +1110,7 @@ in_range_float8_float8(PG_FUNCTION_ARGS)
      * produce a suitably signed infinity, which will compare properly against
      * val whether or not that's infinity.
      */
-    if (sub)
+    else if (sub)
         sum = base - offset;
     else
         sum = base + offset;
@@ -1159,11 +1166,18 @@ in_range_float4_float8(PG_FUNCTION_ARGS)
     /*
      * Deal with infinite offset (necessarily +inf, at this point).  We must
      * special-case this because if base happens to be -inf, their sum would
-     * be NaN, which is an overflow-ish condition we should avoid.
+     * be NaN, which is an overflow-ish condition we must avoid.  We take that
+     * sum to be +inf; that is, we consider base + inf to be +inf for any
+     * non-NaN base.  Conversely, base - inf is taken to be -inf even if base
+     * is +inf.  These choices may seem arbitrary, but they give desirable
+     * windowing behavior with an infinite offset.
      */
     if (isinf(offset))
     {
-        PG_RETURN_BOOL(sub ? !less : less);
+        if (sub)
+            sum = -get_float8_infinity();
+        else
+            sum = get_float8_infinity();
     }

     /*
@@ -1172,7 +1186,7 @@ in_range_float4_float8(PG_FUNCTION_ARGS)
      * produce a suitably signed infinity, which will compare properly against
      * val whether or not that's infinity.
      */
-    if (sub)
+    else if (sub)
         sum = base - offset;
     else
         sum = base + offset;
diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out
index d5fd4045f9..4cb68ca42f 100644
--- a/src/test/regress/expected/window.out
+++ b/src/test/regress/expected/window.out
@@ -1936,6 +1936,42 @@ window w as (order by f_float4 range between
   9 |       NaN |           9 |          9
 (10 rows)

+select id, f_float4, first_value(id) over w, last_value(id) over w
+from numerics
+window w as (order by f_float4 range between
+             'inf' preceding and 'inf' preceding);
+ id | f_float4  | first_value | last_value
+----+-----------+-------------+------------
+  0 | -Infinity |           0 |          0
+  1 |        -3 |           0 |          0
+  2 |        -1 |           0 |          0
+  3 |         0 |           0 |          0
+  4 |       1.1 |           0 |          0
+  5 |      1.12 |           0 |          0
+  6 |         2 |           0 |          0
+  7 |       100 |           0 |          0
+  8 |  Infinity |           0 |          0
+  9 |       NaN |           9 |          9
+(10 rows)
+
+select id, f_float4, first_value(id) over w, last_value(id) over w
+from numerics
+window w as (order by f_float4 range between
+             'inf' following and 'inf' following);
+ id | f_float4  | first_value | last_value
+----+-----------+-------------+------------
+  0 | -Infinity |           8 |          8
+  1 |        -3 |           8 |          8
+  2 |        -1 |           8 |          8
+  3 |         0 |           8 |          8
+  4 |       1.1 |           8 |          8
+  5 |      1.12 |           8 |          8
+  6 |         2 |           8 |          8
+  7 |       100 |           8 |          8
+  8 |  Infinity |           8 |          8
+  9 |       NaN |           9 |          9
+(10 rows)
+
 select id, f_float4, first_value(id) over w, last_value(id) over w
 from numerics
 window w as (order by f_float4 range between
@@ -1995,6 +2031,42 @@ window w as (order by f_float8 range between
   9 |       NaN |           9 |          9
 (10 rows)

+select id, f_float8, first_value(id) over w, last_value(id) over w
+from numerics
+window w as (order by f_float8 range between
+             'inf' preceding and 'inf' preceding);
+ id | f_float8  | first_value | last_value
+----+-----------+-------------+------------
+  0 | -Infinity |           0 |          0
+  1 |        -3 |           0 |          0
+  2 |        -1 |           0 |          0
+  3 |         0 |           0 |          0
+  4 |       1.1 |           0 |          0
+  5 |      1.12 |           0 |          0
+  6 |         2 |           0 |          0
+  7 |       100 |           0 |          0
+  8 |  Infinity |           0 |          0
+  9 |       NaN |           9 |          9
+(10 rows)
+
+select id, f_float8, first_value(id) over w, last_value(id) over w
+from numerics
+window w as (order by f_float8 range between
+             'inf' following and 'inf' following);
+ id | f_float8  | first_value | last_value
+----+-----------+-------------+------------
+  0 | -Infinity |           8 |          8
+  1 |        -3 |           8 |          8
+  2 |        -1 |           8 |          8
+  3 |         0 |           8 |          8
+  4 |       1.1 |           8 |          8
+  5 |      1.12 |           8 |          8
+  6 |         2 |           8 |          8
+  7 |       100 |           8 |          8
+  8 |  Infinity |           8 |          8
+  9 |       NaN |           9 |          9
+(10 rows)
+
 select id, f_float8, first_value(id) over w, last_value(id) over w
 from numerics
 window w as (order by f_float8 range between
diff --git a/src/test/regress/sql/window.sql b/src/test/regress/sql/window.sql
index fe273aa31e..51ec0bac9a 100644
--- a/src/test/regress/sql/window.sql
+++ b/src/test/regress/sql/window.sql
@@ -524,6 +524,14 @@ window w as (order by f_float4 range between
              'inf' preceding and 'inf' following);
 select id, f_float4, first_value(id) over w, last_value(id) over w
 from numerics
+window w as (order by f_float4 range between
+             'inf' preceding and 'inf' preceding);
+select id, f_float4, first_value(id) over w, last_value(id) over w
+from numerics
+window w as (order by f_float4 range between
+             'inf' following and 'inf' following);
+select id, f_float4, first_value(id) over w, last_value(id) over w
+from numerics
 window w as (order by f_float4 range between
              1.1 preceding and 'NaN' following);  -- error, NaN disallowed

@@ -541,6 +549,14 @@ window w as (order by f_float8 range between
              'inf' preceding and 'inf' following);
 select id, f_float8, first_value(id) over w, last_value(id) over w
 from numerics
+window w as (order by f_float8 range between
+             'inf' preceding and 'inf' preceding);
+select id, f_float8, first_value(id) over w, last_value(id) over w
+from numerics
+window w as (order by f_float8 range between
+             'inf' following and 'inf' following);
+select id, f_float8, first_value(id) over w, last_value(id) over w
+from numerics
 window w as (order by f_float8 range between
              1.1 preceding and 'NaN' following);  -- error, NaN disallowed