On Mon, Feb 14, 2022 at 11:23 PM Nathan Bossart
<nathandboss...@gmail.com> wrote:
> It's a little weird
> that justify_hours() and justify_days() can overflow in cases where there
> is still a valid interval representation, but as Tom noted, those functions
> have specific charters to follow.
Yes it is a bit weird, but this follows the same behavior as adding
Intervals. The following query overflows:
postgres=# SELECT interval '2147483647 days' + interval '1 day';
ERROR: interval out of range
Even though the following query does not:
postgres=# SELECT justify_days(interval '2147483647 days') + interval '1 day';
?column?
-----------------------------
5965232 years 4 mons 8 days
(1 row)
The reason is, as Tom mentioned, that Interval's months, days, and
time (microseconds) are stored separately. They are only combined
during certain scenarios such as testing for equality, ordering,
justify_* methods, etc. I think the idea behind it is that not every month
has 30 days and not every day has 24 hrs, though I'm not sure
.
> > + ereport(ERROR,
> > + (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
> > + errmsg("interval out of range")));
>
> nitpick: I think there is ordinarily an extra space before errmsg() so that
> it lines up with errcode().
I've attached a patch to add the space. Thanks so much for your review
and comments!
- Joe Koshakow
From 62e3c824c9243fd53ad95b4d4fc3ef2542d8a941 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <kosh...@gmail.com>
Date: Sun, 13 Feb 2022 13:26:16 -0500
Subject: [PATCH] Check for overflow in justify_interval functions
justify_interval, justify_hours, and justify_days didn't check for
overflow when moving hours to days or days to hours. This commit adds
check for overflow and returns an appropriate error.
Signed-off-by: Joseph Koshakow <kosh...@gmail.com>
---
src/backend/utils/adt/timestamp.c | 29 ++++++++++++++++++---
src/test/regress/expected/interval.out | 36 ++++++++++++++++++++++++++
src/test/regress/sql/interval.sql | 12 +++++++++
3 files changed, 73 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 36f8a84bcc..f45989f807 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -2717,12 +2717,27 @@ interval_justify_interval(PG_FUNCTION_ARGS)
result->day = span->day;
result->time = span->time;
+ /* pre-justify days if it might prevent overflow */
+ if ((result->day > 0 && result->time > 0) ||
+ (result->day < 0 && result->time < 0))
+ {
+ wholemonth = result->day / DAYS_PER_MONTH;
+ result->day -= wholemonth * DAYS_PER_MONTH;
+ if (pg_add_s32_overflow(result->month, wholemonth, &result->month))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
+ }
+
TMODULO(result->time, wholeday, USECS_PER_DAY);
- result->day += wholeday; /* could overflow... */
+ result->day += wholeday;
wholemonth = result->day / DAYS_PER_MONTH;
result->day -= wholemonth * DAYS_PER_MONTH;
- result->month += wholemonth;
+ if (pg_add_s32_overflow(result->month, wholemonth, &result->month))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
if (result->month > 0 &&
(result->day < 0 || (result->day == 0 && result->time < 0)))
@@ -2772,7 +2787,10 @@ interval_justify_hours(PG_FUNCTION_ARGS)
result->time = span->time;
TMODULO(result->time, wholeday, USECS_PER_DAY);
- result->day += wholeday; /* could overflow... */
+ if (pg_add_s32_overflow(result->day, wholeday, &result->day))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
if (result->day > 0 && result->time < 0)
{
@@ -2808,7 +2826,10 @@ interval_justify_days(PG_FUNCTION_ARGS)
wholemonth = result->day / DAYS_PER_MONTH;
result->day -= wholemonth * DAYS_PER_MONTH;
- result->month += wholemonth;
+ if (pg_add_s32_overflow(result->month, wholemonth, &result->month))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
if (result->month > 0 && result->day < 0)
{
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index accd4a7d90..146f7c55d0 100644
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -396,6 +396,10 @@ SELECT justify_days(interval '6 months 36 days 5 hours 4 minutes 3 seconds') as
@ 7 mons 6 days 5 hours 4 mins 3 secs
(1 row)
+SELECT justify_hours(interval '2147483647 days 24 hrs');
+ERROR: interval out of range
+SELECT justify_days(interval '2147483647 months 30 days');
+ERROR: interval out of range
-- test justify_interval()
SELECT justify_interval(interval '1 month -1 hour') as "1 month -1 hour";
1 month -1 hour
@@ -403,6 +407,38 @@ SELECT justify_interval(interval '1 month -1 hour') as "1 month -1 hour";
@ 29 days 23 hours
(1 row)
+SELECT justify_interval(interval '2147483647 days 24 hrs');
+ justify_interval
+-------------------------------
+ @ 5965232 years 4 mons 8 days
+(1 row)
+
+SELECT justify_interval(interval '-2147483648 days -24 hrs');
+ justify_interval
+-----------------------------------
+ @ 5965232 years 4 mons 9 days ago
+(1 row)
+
+SELECT justify_interval(interval '2147483647 months 30 days');
+ERROR: interval out of range
+SELECT justify_interval(interval '-2147483648 months -30 days');
+ERROR: interval out of range
+SELECT justify_interval(interval '2147483647 months 30 days -24 hrs');
+ justify_interval
+----------------------------------
+ @ 178956970 years 7 mons 29 days
+(1 row)
+
+SELECT justify_interval(interval '-2147483648 months -30 days 24 hrs');
+ justify_interval
+--------------------------------------
+ @ 178956970 years 8 mons 29 days ago
+(1 row)
+
+SELECT justify_interval(interval '2147483647 months -30 days 1440 hrs');
+ERROR: interval out of range
+SELECT justify_interval(interval '-2147483648 months 30 days -1440 hrs');
+ERROR: interval out of range
-- test fractional second input, and detection of duplicate units
SET DATESTYLE = 'ISO';
SET IntervalStyle TO postgres;
diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql
index 6d532398bd..c31f0eec05 100644
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -149,10 +149,22 @@ select '100000000y 10mon -1000000000d -100000h -10min -10.000001s ago'::interval
SELECT justify_hours(interval '6 months 3 days 52 hours 3 minutes 2 seconds') as "6 mons 5 days 4 hours 3 mins 2 seconds";
SELECT justify_days(interval '6 months 36 days 5 hours 4 minutes 3 seconds') as "7 mons 6 days 5 hours 4 mins 3 seconds";
+SELECT justify_hours(interval '2147483647 days 24 hrs');
+SELECT justify_days(interval '2147483647 months 30 days');
+
-- test justify_interval()
SELECT justify_interval(interval '1 month -1 hour') as "1 month -1 hour";
+SELECT justify_interval(interval '2147483647 days 24 hrs');
+SELECT justify_interval(interval '-2147483648 days -24 hrs');
+SELECT justify_interval(interval '2147483647 months 30 days');
+SELECT justify_interval(interval '-2147483648 months -30 days');
+SELECT justify_interval(interval '2147483647 months 30 days -24 hrs');
+SELECT justify_interval(interval '-2147483648 months -30 days 24 hrs');
+SELECT justify_interval(interval '2147483647 months -30 days 1440 hrs');
+SELECT justify_interval(interval '-2147483648 months 30 days -1440 hrs');
+
-- test fractional second input, and detection of duplicate units
SET DATESTYLE = 'ISO';
SET IntervalStyle TO postgres;
--
2.25.1
