diff options
| author | Tom Lane | 2017-04-06 03:51:27 +0000 |
|---|---|---|
| committer | Tom Lane | 2017-04-06 03:51:27 +0000 |
| commit | df1a699e5ba3232f373790b2c9485ddf720c4a70 (patch) | |
| tree | cbac578f662225d1497a711d21be51e9a5a8a32c /src/test | |
| parent | 68ea2b7f9b52d35b5fcd9c8d44d88de5a64be3ba (diff) | |
Fix integer-overflow problems in interval comparison.
When using integer timestamps, the interval-comparison functions tried
to compute the overall magnitude of an interval as an int64 number of
microseconds. As reported by Frazer McLean, this overflows for intervals
exceeding about 296000 years, which is bad since we nominally allow
intervals many times larger than that. That results in wrong comparison
results, and possibly in corrupted btree indexes for columns containing
such large interval values.
To fix, compute the magnitude as int128 instead. Although some compilers
have native support for int128 calculations, many don't, so create our
own support functions that can do 128-bit addition and multiplication
if the compiler support isn't there. These support functions are designed
with an eye to allowing the int128 code paths in numeric.c to be rewritten
for use on all platforms, although this patch doesn't do that, or even
provide all the int128 primitives that will be needed for it.
Back-patch as far as 9.4. Earlier releases did not guard against overflow
of interval values at all (commit 146604ec4 fixed that), so it seems not
very exciting to worry about overly-large intervals for them.
Before 9.6, we did not assume that unreferenced "static inline" functions
would not draw compiler warnings, so omit functions not directly referenced
by timestamp.c, the only present consumer of int128.h. (We could have
omitted these functions in HEAD too, but since they were written and
debugged on the way to the present patch, and they look likely to be needed
by numeric.c, let's keep them in HEAD.) I did not bother to try to prevent
such warnings in a --disable-integer-datetimes build, though.
Before 9.5, configure will never define HAVE_INT128, so the part of
int128.h that exploits a native int128 implementation is dead code in the
9.4 branch. I didn't bother to remove it, thinking that keeping the file
looking similar in different branches is more useful.
In HEAD only, add a simple test harness for int128.h in src/tools/.
In back branches, this does not change the float-timestamps code path.
That's not subject to the same kind of overflow risk, since it computes
the interval magnitude as float8. (No doubt, when this code was originally
written, overflow was disregarded for exactly that reason.) There is a
precision hazard instead :-(, but we'll avert our eyes from that question,
since no complaints have been reported and that code's deprecated anyway.
Kyotaro Horiguchi and Tom Lane
Discussion: https://postgr.es/m/1490104629.422698.918452336.26FA96B7@webmail.messagingengine.com
Diffstat (limited to 'src/test')
| -rw-r--r-- | src/test/regress/expected/interval.out | 64 | ||||
| -rw-r--r-- | src/test/regress/sql/interval.sql | 27 |
2 files changed, 91 insertions, 0 deletions
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out index 946c97ad92..f88f34550a 100644 --- a/src/test/regress/expected/interval.out +++ b/src/test/regress/expected/interval.out @@ -207,6 +207,70 @@ SELECT '' AS fortyfive, r1.*, r2.* | 34 years | 6 years (45 rows) +-- Test intervals that are large enough to overflow 64 bits in comparisons +CREATE TEMP TABLE INTERVAL_TBL_OF (f1 interval); +INSERT INTO INTERVAL_TBL_OF (f1) VALUES + ('2147483647 days 2147483647 months'), + ('2147483647 days -2147483648 months'), + ('1 year'), + ('-2147483648 days 2147483647 months'), + ('-2147483648 days -2147483648 months'); +-- these should fail as out-of-range +INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('2147483648 days'); +ERROR: interval field value out of range: "2147483648 days" +LINE 1: INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('2147483648 days'); + ^ +INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483649 days'); +ERROR: interval field value out of range: "-2147483649 days" +LINE 1: INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483649 days')... + ^ +INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('2147483647 years'); +ERROR: interval out of range +LINE 1: INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('2147483647 years')... + ^ +INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483648 years'); +ERROR: interval out of range +LINE 1: INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483648 years'... + ^ +SELECT r1.*, r2.* + FROM INTERVAL_TBL_OF r1, INTERVAL_TBL_OF r2 + WHERE r1.f1 > r2.f1 + ORDER BY r1.f1, r2.f1; + f1 | f1 +-------------------------------------------+------------------------------------------- + -178956970 years -8 mons +2147483647 days | -178956970 years -8 mons -2147483648 days + 1 year | -178956970 years -8 mons -2147483648 days + 1 year | -178956970 years -8 mons +2147483647 days + 178956970 years 7 mons -2147483648 days | -178956970 years -8 mons -2147483648 days + 178956970 years 7 mons -2147483648 days | -178956970 years -8 mons +2147483647 days + 178956970 years 7 mons -2147483648 days | 1 year + 178956970 years 7 mons 2147483647 days | -178956970 years -8 mons -2147483648 days + 178956970 years 7 mons 2147483647 days | -178956970 years -8 mons +2147483647 days + 178956970 years 7 mons 2147483647 days | 1 year + 178956970 years 7 mons 2147483647 days | 178956970 years 7 mons -2147483648 days +(10 rows) + +CREATE INDEX ON INTERVAL_TBL_OF USING btree (f1); +SET enable_seqscan TO false; +EXPLAIN (COSTS OFF) +SELECT f1 FROM INTERVAL_TBL_OF r1 ORDER BY f1; + QUERY PLAN +-------------------------------------------------------------------- + Index Only Scan using interval_tbl_of_f1_idx on interval_tbl_of r1 +(1 row) + +SELECT f1 FROM INTERVAL_TBL_OF r1 ORDER BY f1; + f1 +------------------------------------------- + -178956970 years -8 mons -2147483648 days + -178956970 years -8 mons +2147483647 days + 1 year + 178956970 years 7 mons -2147483648 days + 178956970 years 7 mons 2147483647 days +(5 rows) + +RESET enable_seqscan; +DROP TABLE INTERVAL_TBL_OF; -- Test multiplication and division with intervals. -- Floating point arithmetic rounding errors can lead to unexpected results, -- though the code attempts to do the right thing and round up to days and diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql index cff9adab32..bc5537d1b9 100644 --- a/src/test/regress/sql/interval.sql +++ b/src/test/regress/sql/interval.sql @@ -59,6 +59,33 @@ SELECT '' AS fortyfive, r1.*, r2.* WHERE r1.f1 > r2.f1 ORDER BY r1.f1, r2.f1; +-- Test intervals that are large enough to overflow 64 bits in comparisons +CREATE TEMP TABLE INTERVAL_TBL_OF (f1 interval); +INSERT INTO INTERVAL_TBL_OF (f1) VALUES + ('2147483647 days 2147483647 months'), + ('2147483647 days -2147483648 months'), + ('1 year'), + ('-2147483648 days 2147483647 months'), + ('-2147483648 days -2147483648 months'); +-- these should fail as out-of-range +INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('2147483648 days'); +INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483649 days'); +INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('2147483647 years'); +INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483648 years'); + +SELECT r1.*, r2.* + FROM INTERVAL_TBL_OF r1, INTERVAL_TBL_OF r2 + WHERE r1.f1 > r2.f1 + ORDER BY r1.f1, r2.f1; + +CREATE INDEX ON INTERVAL_TBL_OF USING btree (f1); +SET enable_seqscan TO false; +EXPLAIN (COSTS OFF) +SELECT f1 FROM INTERVAL_TBL_OF r1 ORDER BY f1; +SELECT f1 FROM INTERVAL_TBL_OF r1 ORDER BY f1; +RESET enable_seqscan; + +DROP TABLE INTERVAL_TBL_OF; -- Test multiplication and division with intervals. -- Floating point arithmetic rounding errors can lead to unexpected results, |
