On Fri, Mar 24, 2023 at 9:43 AM Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > > You don't need to do this, but looks like we can add DAYS_PER_WEEK macro and > use it here.
I've attached a patch with this new macro. There's probably tons of places it can be used instead of hardcoding the number 7, but I'll save that for a future patch. - Joe Koshakow
From 41fa5de65c757d72331aff6bb626fab76390e9db Mon Sep 17 00:00:00 2001 From: Joseph Koshakow <kosh...@gmail.com> Date: Sat, 18 Mar 2023 12:26:28 -0400 Subject: [PATCH 1/2] Move integer helper function to int.h --- src/backend/utils/adt/datetime.c | 25 ++++--------------------- src/include/common/int.h | 13 +++++++++++++ 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c index be2e55bb29..64f28a85b0 100644 --- a/src/backend/utils/adt/datetime.c +++ b/src/backend/utils/adt/datetime.c @@ -51,7 +51,6 @@ static int DecodeDate(char *str, int fmask, int *tmask, bool *is2digits, struct pg_tm *tm); static char *AppendSeconds(char *cp, int sec, fsec_t fsec, int precision, bool fillzeros); -static bool int64_multiply_add(int64 val, int64 multiplier, int64 *sum); static bool AdjustFractMicroseconds(double frac, int64 scale, struct pg_itm_in *itm_in); static bool AdjustFractDays(double frac, int scale, @@ -515,22 +514,6 @@ AppendTimestampSeconds(char *cp, struct pg_tm *tm, fsec_t fsec) return AppendSeconds(cp, tm->tm_sec, fsec, MAX_TIMESTAMP_PRECISION, true); } - -/* - * Add val * multiplier to *sum. - * Returns true if successful, false on overflow. - */ -static bool -int64_multiply_add(int64 val, int64 multiplier, int64 *sum) -{ - int64 product; - - if (pg_mul_s64_overflow(val, multiplier, &product) || - pg_add_s64_overflow(*sum, product, sum)) - return false; - return true; -} - /* * Multiply frac by scale (to produce microseconds) and add to itm_in->tm_usec. * Returns true if successful, false if itm_in overflows. @@ -621,7 +604,7 @@ AdjustMicroseconds(int64 val, double fval, int64 scale, struct pg_itm_in *itm_in) { /* Handle the integer part */ - if (!int64_multiply_add(val, scale, &itm_in->tm_usec)) + if (pg_mul_add_s64_overflow(val, scale, &itm_in->tm_usec)) return false; /* Handle the float part */ return AdjustFractMicroseconds(fval, scale, itm_in); @@ -2701,9 +2684,9 @@ DecodeTimeForInterval(char *str, int fmask, int range, return dterr; itm_in->tm_usec = itm.tm_usec; - if (!int64_multiply_add(itm.tm_hour, USECS_PER_HOUR, &itm_in->tm_usec) || - !int64_multiply_add(itm.tm_min, USECS_PER_MINUTE, &itm_in->tm_usec) || - !int64_multiply_add(itm.tm_sec, USECS_PER_SEC, &itm_in->tm_usec)) + if (pg_mul_add_s64_overflow(itm.tm_hour, USECS_PER_HOUR, &itm_in->tm_usec) || + pg_mul_add_s64_overflow(itm.tm_min, USECS_PER_MINUTE, &itm_in->tm_usec) || + pg_mul_add_s64_overflow(itm.tm_sec, USECS_PER_SEC, &itm_in->tm_usec)) return DTERR_FIELD_OVERFLOW; return 0; diff --git a/src/include/common/int.h b/src/include/common/int.h index 450800894e..81726c65f7 100644 --- a/src/include/common/int.h +++ b/src/include/common/int.h @@ -254,6 +254,19 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result) #endif } +/* + * Add val * multiplier to *sum. + * Returns false if successful, true on overflow. + */ +static inline bool +pg_mul_add_s64_overflow(int64 val, int64 multiplier, int64 *sum) +{ + int64 product; + + return pg_mul_s64_overflow(val, multiplier, &product) || + pg_add_s64_overflow(*sum, product, sum); +} + /*------------------------------------------------------------------------ * Overflow routines for unsigned integers *------------------------------------------------------------------------ -- 2.34.1
From 242ffd232bef606c9c948f0ee9980152fb9e3bec Mon Sep 17 00:00:00 2001 From: Joseph Koshakow <kosh...@gmail.com> Date: Sat, 18 Mar 2023 12:38:58 -0400 Subject: [PATCH 2/2] Check for overflow in make_interval --- src/backend/utils/adt/timestamp.c | 24 +++++++++++++++++++----- src/include/common/int.h | 13 +++++++++++++ src/include/datatype/timestamp.h | 1 + src/test/regress/expected/interval.out | 5 +++++ src/test/regress/sql/interval.sql | 4 ++++ 5 files changed, 42 insertions(+), 5 deletions(-) diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c index aaadc68ae6..ccf0019a3c 100644 --- a/src/backend/utils/adt/timestamp.c +++ b/src/backend/utils/adt/timestamp.c @@ -1517,13 +1517,27 @@ make_interval(PG_FUNCTION_ARGS) errmsg("interval out of range"))); result = (Interval *) palloc(sizeof(Interval)); - result->month = years * MONTHS_PER_YEAR + months; - result->day = weeks * 7 + days; + result->month = months; + if (pg_mul_add_s32_overflow(years, MONTHS_PER_YEAR, &result->month)) + ereport(ERROR, + (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE), + errmsg("interval out of range"))); + result->day = days; + if (pg_mul_add_s32_overflow(weeks, DAYS_PER_WEEK, &result->day)) + ereport(ERROR, + (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE), + errmsg("interval out of range"))); secs = rint(secs * USECS_PER_SEC); - result->time = hours * ((int64) SECS_PER_HOUR * USECS_PER_SEC) + - mins * ((int64) SECS_PER_MINUTE * USECS_PER_SEC) + - (int64) secs; + result->time = secs; + if (pg_mul_add_s64_overflow(mins, ((int64) SECS_PER_MINUTE * USECS_PER_SEC), &result->time)) + ereport(ERROR, + (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE), + errmsg("interval out of range"))); + if (pg_mul_add_s64_overflow(hours, ((int64) SECS_PER_HOUR * USECS_PER_SEC), &result->time)) + ereport(ERROR, + (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE), + errmsg("interval out of range"))); PG_RETURN_INTERVAL_P(result); } diff --git a/src/include/common/int.h b/src/include/common/int.h index 81726c65f7..48ef495551 100644 --- a/src/include/common/int.h +++ b/src/include/common/int.h @@ -154,6 +154,19 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result) #endif } +/* + * Add val * multiplier to *sum. + * Returns false if successful, true on overflow. + */ +static inline bool +pg_mul_add_s32_overflow(int32 val, int32 multiplier, int32 *sum) +{ + int32 product; + + return pg_mul_s32_overflow(val, multiplier, &product) || + pg_add_s32_overflow(*sum, product, sum); +} + /* * INT64 */ diff --git a/src/include/datatype/timestamp.h b/src/include/datatype/timestamp.h index ab8ccf89ca..1a6390585c 100644 --- a/src/include/datatype/timestamp.h +++ b/src/include/datatype/timestamp.h @@ -114,6 +114,7 @@ struct pg_itm_in * 30 days. */ #define DAYS_PER_MONTH 30 /* assumes exactly 30 days per month */ +#define DAYS_PER_WEEK 7 #define HOURS_PER_DAY 24 /* assume no daylight savings time changes */ /* diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out index 28b71d9681..27bfb8ba9b 100644 --- a/src/test/regress/expected/interval.out +++ b/src/test/regress/expected/interval.out @@ -1587,6 +1587,11 @@ select interval '-2147483648 months -2147483648 days -9223372036854775808 micros ERROR: interval field value out of range: "-2147483648 months -2147483648 days -9223372036854775808 microseconds ago" LINE 1: select interval '-2147483648 months -2147483648 days -922337... ^ +-- overflowing using make_interval +select make_interval(1, 2147483647, 2147483647, 1, 1, 1, 9223372036854.7759); +ERROR: interval out of range +select make_interval(-1, -2147483648, -2147483648, -1, -1, -1, -9223372036854.7759); +ERROR: interval out of range -- test that INT_MIN number is formatted properly SET IntervalStyle to postgres; select interval '-2147483648 months -2147483648 days -9223372036854775808 us'; diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql index 56feda1a3d..f1abf08501 100644 --- a/src/test/regress/sql/interval.sql +++ b/src/test/regress/sql/interval.sql @@ -511,6 +511,10 @@ select interval '-2147483648 days ago'; select interval '-9223372036854775808 microseconds ago'; select interval '-2147483648 months -2147483648 days -9223372036854775808 microseconds ago'; +-- overflowing using make_interval +select make_interval(1, 2147483647, 2147483647, 1, 1, 1, 9223372036854.7759); +select make_interval(-1, -2147483648, -2147483648, -1, -1, -1, -9223372036854.7759); + -- test that INT_MIN number is formatted properly SET IntervalStyle to postgres; select interval '-2147483648 months -2147483648 days -9223372036854775808 us'; -- 2.34.1