Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit a4faef8

Browse files
committed
Fix some corner cases for window ranges with infinite offsets.
Many situations where the offset is infinity were not handled sanely. We should generally allow the val versus base +/- offset comparison to proceed according to the normal rules of IEEE arithmetic; however, we must do something special for the corner cases where base +/- offset would produce NaN due to subtracting two like-signed infinities. That corresponds to asking which values infinitely precede +inf or infinitely follow -inf, which should certainly be true of any finite value or of the opposite-signed infinity. After some discussion it seems that the best decision is to make it true of the same-signed infinity as well, ie, just return constant TRUE if the calculation would produce a NaN. (We could write this with a bit less code by subtracting anyway, and then checking for a NaN result. However, I prefer this formulation because it'll be easier to transpose into numeric.c.) Although this seems like clearly a bug fix with respect to finite values, it is less obviously correct for infinite values. Between that and the fact that the whole issue only arises for very strange window specifications (e.g. RANGE BETWEEN 'inf' PRECEDING AND 'inf' PRECEDING), I'll desist from back-patching. Noted by Dean Rasheed. Discussion: https://postgr.es/m/3393130.1594925893@sss.pgh.pa.us
1 parent 4fb6aeb commit a4faef8

File tree

3 files changed

+118
-16
lines changed

3 files changed

+118
-16
lines changed

src/backend/utils/adt/float.c

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,18 +1088,25 @@ in_range_float8_float8(PG_FUNCTION_ARGS)
10881088
}
10891089

10901090
/*
1091-
* Deal with infinite offset (necessarily +inf, at this point). We must
1092-
* special-case this because if base happens to be -inf, their sum would
1093-
* be NaN, which is an overflow-ish condition we should avoid.
1091+
* Deal with cases where both base and offset are infinite, and computing
1092+
* base +/- offset would produce NaN. This corresponds to a window frame
1093+
* whose boundary infinitely precedes +inf or infinitely follows -inf,
1094+
* which is not well-defined. For consistency with other cases involving
1095+
* infinities, such as the fact that +inf infinitely follows +inf, we
1096+
* choose to assume that +inf infinitely precedes +inf and -inf infinitely
1097+
* follows -inf, and therefore that all finite and infinite values are in
1098+
* such a window frame.
1099+
*
1100+
* offset is known positive, so we need only check the sign of base in
1101+
* this test.
10941102
*/
1095-
if (isinf(offset))
1096-
{
1097-
PG_RETURN_BOOL(sub ? !less : less);
1098-
}
1103+
if (isinf(offset) && isinf(base) &&
1104+
(sub ? base > 0 : base < 0))
1105+
PG_RETURN_BOOL(true);
10991106

11001107
/*
11011108
* Otherwise it should be safe to compute base +/- offset. We trust the
1102-
* FPU to cope if base is +/-inf or the true sum would overflow, and
1109+
* FPU to cope if an input is +/-inf or the true sum would overflow, and
11031110
* produce a suitably signed infinity, which will compare properly against
11041111
* val whether or not that's infinity.
11051112
*/
@@ -1157,18 +1164,25 @@ in_range_float4_float8(PG_FUNCTION_ARGS)
11571164
}
11581165

11591166
/*
1160-
* Deal with infinite offset (necessarily +inf, at this point). We must
1161-
* special-case this because if base happens to be -inf, their sum would
1162-
* be NaN, which is an overflow-ish condition we should avoid.
1167+
* Deal with cases where both base and offset are infinite, and computing
1168+
* base +/- offset would produce NaN. This corresponds to a window frame
1169+
* whose boundary infinitely precedes +inf or infinitely follows -inf,
1170+
* which is not well-defined. For consistency with other cases involving
1171+
* infinities, such as the fact that +inf infinitely follows +inf, we
1172+
* choose to assume that +inf infinitely precedes +inf and -inf infinitely
1173+
* follows -inf, and therefore that all finite and infinite values are in
1174+
* such a window frame.
1175+
*
1176+
* offset is known positive, so we need only check the sign of base in
1177+
* this test.
11631178
*/
1164-
if (isinf(offset))
1165-
{
1166-
PG_RETURN_BOOL(sub ? !less : less);
1167-
}
1179+
if (isinf(offset) && isinf(base) &&
1180+
(sub ? base > 0 : base < 0))
1181+
PG_RETURN_BOOL(true);
11681182

11691183
/*
11701184
* Otherwise it should be safe to compute base +/- offset. We trust the
1171-
* FPU to cope if base is +/-inf or the true sum would overflow, and
1185+
* FPU to cope if an input is +/-inf or the true sum would overflow, and
11721186
* produce a suitably signed infinity, which will compare properly against
11731187
* val whether or not that's infinity.
11741188
*/

src/test/regress/expected/window.out

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1936,6 +1936,42 @@ window w as (order by f_float4 range between
19361936
9 | NaN | 9 | 9
19371937
(10 rows)
19381938

1939+
select id, f_float4, first_value(id) over w, last_value(id) over w
1940+
from numerics
1941+
window w as (order by f_float4 range between
1942+
'inf' preceding and 'inf' preceding);
1943+
id | f_float4 | first_value | last_value
1944+
----+-----------+-------------+------------
1945+
0 | -Infinity | 0 | 0
1946+
1 | -3 | 0 | 0
1947+
2 | -1 | 0 | 0
1948+
3 | 0 | 0 | 0
1949+
4 | 1.1 | 0 | 0
1950+
5 | 1.12 | 0 | 0
1951+
6 | 2 | 0 | 0
1952+
7 | 100 | 0 | 0
1953+
8 | Infinity | 0 | 8
1954+
9 | NaN | 9 | 9
1955+
(10 rows)
1956+
1957+
select id, f_float4, first_value(id) over w, last_value(id) over w
1958+
from numerics
1959+
window w as (order by f_float4 range between
1960+
'inf' following and 'inf' following);
1961+
id | f_float4 | first_value | last_value
1962+
----+-----------+-------------+------------
1963+
0 | -Infinity | 0 | 8
1964+
1 | -3 | 8 | 8
1965+
2 | -1 | 8 | 8
1966+
3 | 0 | 8 | 8
1967+
4 | 1.1 | 8 | 8
1968+
5 | 1.12 | 8 | 8
1969+
6 | 2 | 8 | 8
1970+
7 | 100 | 8 | 8
1971+
8 | Infinity | 8 | 8
1972+
9 | NaN | 9 | 9
1973+
(10 rows)
1974+
19391975
select id, f_float4, first_value(id) over w, last_value(id) over w
19401976
from numerics
19411977
window w as (order by f_float4 range between
@@ -1995,6 +2031,42 @@ window w as (order by f_float8 range between
19952031
9 | NaN | 9 | 9
19962032
(10 rows)
19972033

2034+
select id, f_float8, first_value(id) over w, last_value(id) over w
2035+
from numerics
2036+
window w as (order by f_float8 range between
2037+
'inf' preceding and 'inf' preceding);
2038+
id | f_float8 | first_value | last_value
2039+
----+-----------+-------------+------------
2040+
0 | -Infinity | 0 | 0
2041+
1 | -3 | 0 | 0
2042+
2 | -1 | 0 | 0
2043+
3 | 0 | 0 | 0
2044+
4 | 1.1 | 0 | 0
2045+
5 | 1.12 | 0 | 0
2046+
6 | 2 | 0 | 0
2047+
7 | 100 | 0 | 0
2048+
8 | Infinity | 0 | 8
2049+
9 | NaN | 9 | 9
2050+
(10 rows)
2051+
2052+
select id, f_float8, first_value(id) over w, last_value(id) over w
2053+
from numerics
2054+
window w as (order by f_float8 range between
2055+
'inf' following and 'inf' following);
2056+
id | f_float8 | first_value | last_value
2057+
----+-----------+-------------+------------
2058+
0 | -Infinity | 0 | 8
2059+
1 | -3 | 8 | 8
2060+
2 | -1 | 8 | 8
2061+
3 | 0 | 8 | 8
2062+
4 | 1.1 | 8 | 8
2063+
5 | 1.12 | 8 | 8
2064+
6 | 2 | 8 | 8
2065+
7 | 100 | 8 | 8
2066+
8 | Infinity | 8 | 8
2067+
9 | NaN | 9 | 9
2068+
(10 rows)
2069+
19982070
select id, f_float8, first_value(id) over w, last_value(id) over w
19992071
from numerics
20002072
window w as (order by f_float8 range between

src/test/regress/sql/window.sql

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,14 @@ window w as (order by f_float4 range between
524524
'inf' preceding and 'inf' following);
525525
select id, f_float4, first_value(id) over w, last_value(id) over w
526526
from numerics
527+
window w as (order by f_float4 range between
528+
'inf' preceding and 'inf' preceding);
529+
select id, f_float4, first_value(id) over w, last_value(id) over w
530+
from numerics
531+
window w as (order by f_float4 range between
532+
'inf' following and 'inf' following);
533+
select id, f_float4, first_value(id) over w, last_value(id) over w
534+
from numerics
527535
window w as (order by f_float4 range between
528536
1.1 preceding and 'NaN' following); -- error, NaN disallowed
529537

@@ -541,6 +549,14 @@ window w as (order by f_float8 range between
541549
'inf' preceding and 'inf' following);
542550
select id, f_float8, first_value(id) over w, last_value(id) over w
543551
from numerics
552+
window w as (order by f_float8 range between
553+
'inf' preceding and 'inf' preceding);
554+
select id, f_float8, first_value(id) over w, last_value(id) over w
555+
from numerics
556+
window w as (order by f_float8 range between
557+
'inf' following and 'inf' following);
558+
select id, f_float8, first_value(id) over w, last_value(id) over w
559+
from numerics
544560
window w as (order by f_float8 range between
545561
1.1 preceding and 'NaN' following); -- error, NaN disallowed
546562

0 commit comments

Comments
 (0)