INTERVAL overflow detection is terribly broken
Hi, after studying ITERVAL and having a long chat with RhoidumToad and
StuckMojo on #postgresql, I am presenting you 3 bugs regarding INTERVAL.
As far as I understand, the Interval struct (binary internal
representation) consists of:
int32 months
int32 days
int64 microseconds
1. OUTPUT ERRORS: Since 2^63 microseconds equals 2,562,047,788 > 2^31
hours, the overflow in pg_tm when displaying the value causes overflow. The
value of Interval struct is actually correct, error happens only on
displaying it.
SELECT INTERVAL '2147483647 hours' + INTERVAL '5 hours'
"-2147483644:00:00"
Even wireder:
SELECT INTERVAL '2147483647 hours' + '1 hour'
"--2147483648:00:00"
notice the double minus? Don't ask how I came across this two bugs.
2. OPERATION ERRORS: When summing two intervals, the user is not notified
when overflow occurs:
SELECT INT '2147483647' + INT '1'
ERROR: integer out of range
SELECT INTERVAL '2147483647 days' + INTERVAL '1 day'
"-2147483648 days"
This should be analogous.
3. PARSER / INPUT ERRORS:
This is perhaps the hardest one to explain, since this is an architectural
flaw. You are checking the overflows when parsing string -> pg_tm struct.
However, at this point, the parser doesn't know, that weeks and days are
going to get combined, or years are going to get converted to months, for
example.
Unawarness of underlying Interval struct causes two types of suberrors:
a) False positive
SELECT INTERVAL '2147483648 microseconds'
ERROR: interval field value out of range: "2147483648 microseconds"
This is not right. Microseconds are internally stored as 64 bit signed
integer. The catch is: this amount of microseconds is representable in
Interval data structure, this shouldn't be an error.
b) False negative
SELECT INTERVAL '1000000000 years'
"-73741824 years"
We don't catch errors like this, because parser only checks for overflow in
pg_tm. If overflow laters happens in Interval, we don't seem to care.
4. POSSIBLE SOLUTIONS:
a) Move the overflow checking just after the conversion of pg_tm ->
Interval is made. This way, you can accurately predict if the result is
really not store-able.
b) Because of 1), you have to declare tm_hour as int64, if you want to use
that for the output. But, why not use Interval struct for printing
directly, without intermediate pg_tm?
5. Offtopic: Correct the documentation, INTERVAL data size is 16 bytes, not
12.
Rok Kralj
--
eMail: rok.kralj@gmail.com
So, any insights on these problems?
They might not be critical, but might be silently corrupting someone's data.
2013/6/23 Rok Kralj <rok.kralj@gmail.com>
Hi, after studying ITERVAL and having a long chat with RhoidumToad and
StuckMojo on #postgresql, I am presenting you 3 bugs regarding INTERVAL.As far as I understand, the Interval struct (binary internal
representation) consists of:int32 months
int32 days
int64 microseconds1. OUTPUT ERRORS: Since 2^63 microseconds equals 2,562,047,788 > 2^31
hours, the overflow in pg_tm when displaying the value causes overflow. The
value of Interval struct is actually correct, error happens only on
displaying it.SELECT INTERVAL '2147483647 hours' + INTERVAL '5 hours'
"-2147483644:00:00"Even wireder:
SELECT INTERVAL '2147483647 hours' + '1 hour'
"--2147483648:00:00"notice the double minus? Don't ask how I came across this two bugs.
2. OPERATION ERRORS: When summing two intervals, the user is not notified
when overflow occurs:SELECT INT '2147483647' + INT '1'
ERROR: integer out of rangeSELECT INTERVAL '2147483647 days' + INTERVAL '1 day'
"-2147483648 days"This should be analogous.
3. PARSER / INPUT ERRORS:
This is perhaps the hardest one to explain, since this is an architectural
flaw. You are checking the overflows when parsing string -> pg_tm struct.
However, at this point, the parser doesn't know, that weeks and days are
going to get combined, or years are going to get converted to months, for
example.Unawarness of underlying Interval struct causes two types of suberrors:
a) False positive
SELECT INTERVAL '2147483648 microseconds'
ERROR: interval field value out of range: "2147483648 microseconds"This is not right. Microseconds are internally stored as 64 bit signed
integer. The catch is: this amount of microseconds is representable in
Interval data structure, this shouldn't be an error.b) False negative
SELECT INTERVAL '1000000000 years'
"-73741824 years"We don't catch errors like this, because parser only checks for overflow
in pg_tm. If overflow laters happens in Interval, we don't seem to care.4. POSSIBLE SOLUTIONS:
a) Move the overflow checking just after the conversion of pg_tm ->
Interval is made. This way, you can accurately predict if the result is
really not store-able.b) Because of 1), you have to declare tm_hour as int64, if you want to use
that for the output. But, why not use Interval struct for printing
directly, without intermediate pg_tm?5. Offtopic: Correct the documentation, INTERVAL data size is 16 bytes,
not 12.Rok Kralj
--
eMail: rok.kralj@gmail.com
--
eMail: rok.kralj@gmail.com
On Sun, Jun 23, 2013 at 03:00:59PM +0200, Rok Kralj wrote:
Hi, after studying ITERVAL and having a long chat with RhoidumToad and
StuckMojo on #postgresql, I am presenting you 3 bugs regarding INTERVAL.
OK, I am going to merge this with the previous report/patch which fixes:
SELECT INTERVAL '2000000000 years';
ERROR: interval out of range
LINE 1: SELECT INTERVAL '2000000000 years';
and
SELECT INTERVAL '2000000000-3 years';
ERROR: interval field value out of range: "2000000000-3 years"
LINE 1: SELECT INTERVAL '2000000000-3 years';
As far as I understand, the Interval struct (binary internal representation)
consists of:int32 months
int32 days
int64 microseconds1. OUTPUT ERRORS: Since 2^63 microseconds equals�2,562,047,788 > 2^31 hours,
the overflow in pg_tm when displaying the value causes overflow. The value of
Interval struct is actually correct, error happens only on displaying it.SELECT INTERVAL '2147483647 hours' + INTERVAL '5 hours'
"-2147483644:00:00"
Fixed:
SELECT INTERVAL '2147483647 hours' + INTERVAL '5 hours';
ERROR: interval out of range
Even wireder:
SELECT INTERVAL '2147483647 hours' + '1 hour'
"--2147483648:00:00"
Fixed:
SELECT INTERVAL '2147483647 hours' + '1 hour';
ERROR: interval out of range
notice the double minus? Don't ask how I came across this two bugs.
2. OPERATION ERRORS: When summing two intervals, the user is not notified when
overflow occurs:SELECT INT '2147483647' + INT '1'
ERROR: integer out of rangeSELECT INTERVAL '2147483647 days' + INTERVAL '1 day'
"-2147483648 days"This should be analogous.
Fixed:
SELECT INTERVAL '2147483647 days' + INTERVAL '1 day';
ERROR: interval out of range
3. PARSER / INPUT ERRORS:
This is perhaps the hardest one to explain, since this is an architectural
flaw. You are checking the overflows when parsing string -> pg_tm struct.
However, at this point, the parser doesn't know, that weeks and days are going
to get combined, or years are going to get converted to months, for example.Unawarness of underlying Interval struct causes two types of suberrors:
a) False positive
SELECT INTERVAL '2147483648 microseconds'
ERROR: �interval field value out of range: "2147483648 microseconds"This is not right. Microseconds are internally stored as 64 bit signed integer.
The catch is: this amount of microseconds is representable in Interval data
structure, this shouldn't be an error.
I don't see a way to fix this as we are testing too early to know what
type of value it is, as you stated.
b) False negative
SELECT INTERVAL '1000000000 years'
"-73741824 years"We don't catch errors like this, because parser only checks for overflow in
pg_tm. If overflow laters happens in Interval, we don't seem to care.
Fixed:
SELECT INTERVAL '1000000000 years';
ERROR: interval out of range
LINE 1: SELECT INTERVAL '1000000000 years';
4. POSSIBLE SOLUTIONS:
a) Move the overflow checking just after the conversion of pg_tm -> Interval is
made. This way, you can accurately predict if the result is really not
store-able.b) Because of 1), you have to declare tm_hour as int64, if you want to use that
for the output. But, why not use Interval struct for printing directly, without
intermediate pg_tm?5. Offtopic: Correct the documentation, INTERVAL data size is 16 bytes, not 12.
Fixed.
Patch attached.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
Attachments:
interval.difftext/x-diff; charset=us-asciiDownload
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
new file mode 100644
index 6bf4cf6..b7d7d80
*** a/doc/src/sgml/datatype.sgml
--- b/doc/src/sgml/datatype.sgml
*************** SELECT E'\\xDEADBEEF';
*** 1587,1593 ****
</row>
<row>
<entry><type>interval [ <replaceable>fields</replaceable> ] [ (<replaceable>p</replaceable>) ]</type></entry>
! <entry>12 bytes</entry>
<entry>time interval</entry>
<entry>-178000000 years</entry>
<entry>178000000 years</entry>
--- 1587,1593 ----
</row>
<row>
<entry><type>interval [ <replaceable>fields</replaceable> ] [ (<replaceable>p</replaceable>) ]</type></entry>
! <entry>16 bytes</entry>
<entry>time interval</entry>
<entry>-178000000 years</entry>
<entry>178000000 years</entry>
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
new file mode 100644
index a61b40e..ae4e9e7
*** a/src/backend/utils/adt/datetime.c
--- b/src/backend/utils/adt/datetime.c
*************** DecodeInterval(char **field, int *ftype,
*** 2976,2981 ****
--- 2976,2983 ----
type = DTK_MONTH;
if (*field[i] == '-')
val2 = -val2;
+ if (((float)val * MONTHS_PER_YEAR + val2) > INT_MAX)
+ return DTERR_FIELD_OVERFLOW;
val = val * MONTHS_PER_YEAR + val2;
fval = 0;
}
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
new file mode 100644
index 4581862..8b28b63
*** a/src/backend/utils/adt/timestamp.c
--- b/src/backend/utils/adt/timestamp.c
***************
*** 41,46 ****
--- 41,47 ----
#error -ffast-math is known to break this code
#endif
+ #define SAMESIGN(a,b) (((a) < 0) == ((b) < 0))
/* Set at postmaster start */
TimestampTz PgStartTime;
*************** interval2tm(Interval span, struct pg_tm
*** 1694,1700 ****
#ifdef HAVE_INT64_TIMESTAMP
tfrac = time / USECS_PER_HOUR;
time -= tfrac * USECS_PER_HOUR;
! tm->tm_hour = tfrac; /* could overflow ... */
tfrac = time / USECS_PER_MINUTE;
time -= tfrac * USECS_PER_MINUTE;
tm->tm_min = tfrac;
--- 1695,1705 ----
#ifdef HAVE_INT64_TIMESTAMP
tfrac = time / USECS_PER_HOUR;
time -= tfrac * USECS_PER_HOUR;
! tm->tm_hour = tfrac;
! if (!SAMESIGN(tm->tm_hour, tfrac))
! ereport(ERROR,
! (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
! errmsg("interval out of range")));
tfrac = time / USECS_PER_MINUTE;
time -= tfrac * USECS_PER_MINUTE;
tm->tm_min = tfrac;
*************** recalc:
*** 1725,1730 ****
--- 1730,1737 ----
int
tm2interval(struct pg_tm * tm, fsec_t fsec, Interval *span)
{
+ if ((float)tm->tm_year * MONTHS_PER_YEAR + tm->tm_mon > INT_MAX)
+ return -1;
span->month = tm->tm_year * MONTHS_PER_YEAR + tm->tm_mon;
span->day = tm->tm_mday;
#ifdef HAVE_INT64_TIMESTAMP
*************** interval_pl(PG_FUNCTION_ARGS)
*** 2872,2879 ****
--- 2879,2903 ----
result = (Interval *) palloc(sizeof(Interval));
result->month = span1->month + span2->month;
+ if (SAMESIGN(span1->month, span2->month) &&
+ !SAMESIGN(result->month, span1->month))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
+
result->day = span1->day + span2->day;
+ if (SAMESIGN(span1->day, span2->day) &&
+ !SAMESIGN(result->day, span1->day))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
+
result->time = span1->time + span2->time;
+ if (SAMESIGN(span1->time, span2->time) &&
+ !SAMESIGN(result->time, span1->time))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
PG_RETURN_INTERVAL_P(result);
}
diff --git a/src/interfaces/ecpg/pgtypeslib/interval.c b/src/interfaces/ecpg/pgtypeslib/interval.c
new file mode 100644
index efa775d..e1307e3
*** a/src/interfaces/ecpg/pgtypeslib/interval.c
--- b/src/interfaces/ecpg/pgtypeslib/interval.c
*************** recalc:
*** 1036,1041 ****
--- 1036,1043 ----
static int
tm2interval(struct tm * tm, fsec_t fsec, interval * span)
{
+ if ((float)tm->tm_year * MONTHS_PER_YEAR + tm->tm_mon > INT_MAX)
+ return -1;
span->month = tm->tm_year * MONTHS_PER_YEAR + tm->tm_mon;
#ifdef HAVE_INT64_TIMESTAMP
span->time = (((((((tm->tm_mday * INT64CONST(24)) +
On Jan26, 2014, at 03:50 , Bruce Momjian <bruce@momjian.us> wrote:
Patch attached.
+ if ((float)tm->tm_year * MONTHS_PER_YEAR + tm->tm_mon > INT_MAX) + return -1;
Is this bullet-proof? If float and int are both 32-bit, the float's mantissa
will be less than 32-bit (24 or so, I think), and thus won't be able to
represent INT_MAX accurately. This means there's a value of
tm_year * MONTHS_PER_YEAR which is less than INT_MAX, yet for which
tm_year * MONTHS_PER_YEAR + tm_mon will return tm_year * MONTHS_PER_YEAR
unmodified if tm_mon is small enough (e.g, 1). But if tm_year * MONTHS_PER_YEAR
was close enough to INT_MAX, the actual value of
tm_year * MONTHS_PER_YEAR + tm_mon might still be larger than INT_MAX,
the floating-point computation just won't detect it. In that case, the
check would succeed, yet the actual integer computation would still overflow.
It should be safe with "double" instead of "float".
+ if (SAMESIGN(span1->month, span2->month) && + !SAMESIGN(result->month, span1->month))
This assumes wrapping semantics for signed integral types, which isn't
guaranteed by the C standard - it says overflows of signed integral types
produce undefined results. We currently depend on wrapping semantics for
these types in more places, and therefore need GCC's "-fwrapv" anway, but
I still wonder if adding more of these kinds of checks is a good idea.
best regards,
Florian Pflug
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Jan 26, 2014 at 02:13:03PM +0100, Florian Pflug wrote:
On Jan26, 2014, at 03:50 , Bruce Momjian <bruce@momjian.us> wrote:
Patch attached.
+ if ((float)tm->tm_year * MONTHS_PER_YEAR + tm->tm_mon > INT_MAX) + return -1;Is this bullet-proof? If float and int are both 32-bit, the float's mantissa
will be less than 32-bit (24 or so, I think), and thus won't be able to
represent INT_MAX accurately. This means there's a value of
tm_year * MONTHS_PER_YEAR which is less than INT_MAX, yet for which
tm_year * MONTHS_PER_YEAR + tm_mon will return tm_year * MONTHS_PER_YEAR
unmodified if tm_mon is small enough (e.g, 1). But if tm_year * MONTHS_PER_YEAR
was close enough to INT_MAX, the actual value of
tm_year * MONTHS_PER_YEAR + tm_mon might still be larger than INT_MAX,
the floating-point computation just won't detect it. In that case, the
check would succeed, yet the actual integer computation would still overflow.It should be safe with "double" instead of "float".
You are correct. I have adjusted the code to use "double".
+ if (SAMESIGN(span1->month, span2->month) && + !SAMESIGN(result->month, span1->month))This assumes wrapping semantics for signed integral types, which isn't
guaranteed by the C standard - it says overflows of signed integral types
produce undefined results. We currently depend on wrapping semantics for
these types in more places, and therefore need GCC's "-fwrapv" anway, but
I still wonder if adding more of these kinds of checks is a good idea.
Well, I copied this from int.c, so what I did was to mention the other
function I got this code from. If we ever change int.c we would need to
change this too.
The updated attached patch has overflow detection for interval
subtraction, multiply, and negate. There are also some comparison
cleanups.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
Attachments:
interval.difftext/x-diff; charset=us-asciiDownload
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
new file mode 100644
index 6bf4cf6..b7d7d80
*** a/doc/src/sgml/datatype.sgml
--- b/doc/src/sgml/datatype.sgml
*************** SELECT E'\\xDEADBEEF';
*** 1587,1593 ****
</row>
<row>
<entry><type>interval [ <replaceable>fields</replaceable> ] [ (<replaceable>p</replaceable>) ]</type></entry>
! <entry>12 bytes</entry>
<entry>time interval</entry>
<entry>-178000000 years</entry>
<entry>178000000 years</entry>
--- 1587,1593 ----
</row>
<row>
<entry><type>interval [ <replaceable>fields</replaceable> ] [ (<replaceable>p</replaceable>) ]</type></entry>
! <entry>16 bytes</entry>
<entry>time interval</entry>
<entry>-178000000 years</entry>
<entry>178000000 years</entry>
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
new file mode 100644
index a61b40e..70284e9
*** a/src/backend/utils/adt/datetime.c
--- b/src/backend/utils/adt/datetime.c
*************** DecodeInterval(char **field, int *ftype,
*** 2976,2981 ****
--- 2976,2983 ----
type = DTK_MONTH;
if (*field[i] == '-')
val2 = -val2;
+ if (((double)val * MONTHS_PER_YEAR + val2) > INT_MAX)
+ return DTERR_FIELD_OVERFLOW;
val = val * MONTHS_PER_YEAR + val2;
fval = 0;
}
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
new file mode 100644
index 4581862..3b8e8e8
*** a/src/backend/utils/adt/timestamp.c
--- b/src/backend/utils/adt/timestamp.c
***************
*** 41,46 ****
--- 41,47 ----
#error -ffast-math is known to break this code
#endif
+ #define SAMESIGN(a,b) (((a) < 0) == ((b) < 0))
/* Set at postmaster start */
TimestampTz PgStartTime;
*************** interval2tm(Interval span, struct pg_tm
*** 1694,1700 ****
#ifdef HAVE_INT64_TIMESTAMP
tfrac = time / USECS_PER_HOUR;
time -= tfrac * USECS_PER_HOUR;
! tm->tm_hour = tfrac; /* could overflow ... */
tfrac = time / USECS_PER_MINUTE;
time -= tfrac * USECS_PER_MINUTE;
tm->tm_min = tfrac;
--- 1695,1705 ----
#ifdef HAVE_INT64_TIMESTAMP
tfrac = time / USECS_PER_HOUR;
time -= tfrac * USECS_PER_HOUR;
! tm->tm_hour = tfrac;
! if (!SAMESIGN(tm->tm_hour, tfrac))
! ereport(ERROR,
! (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
! errmsg("interval out of range")));
tfrac = time / USECS_PER_MINUTE;
time -= tfrac * USECS_PER_MINUTE;
tm->tm_min = tfrac;
*************** recalc:
*** 1725,1731 ****
int
tm2interval(struct pg_tm * tm, fsec_t fsec, Interval *span)
{
! span->month = tm->tm_year * MONTHS_PER_YEAR + tm->tm_mon;
span->day = tm->tm_mday;
#ifdef HAVE_INT64_TIMESTAMP
span->time = (((((tm->tm_hour * INT64CONST(60)) +
--- 1730,1740 ----
int
tm2interval(struct pg_tm * tm, fsec_t fsec, Interval *span)
{
! double total_months = (double)tm->tm_year * MONTHS_PER_YEAR + tm->tm_mon;
!
! if (total_months > INT_MAX || total_months < INT_MIN)
! return -1;
! span->month = total_months;
span->day = tm->tm_mday;
#ifdef HAVE_INT64_TIMESTAMP
span->time = (((((tm->tm_hour * INT64CONST(60)) +
*************** interval_um(PG_FUNCTION_ARGS)
*** 2826,2833 ****
--- 2835,2855 ----
result = (Interval *) palloc(sizeof(Interval));
result->time = -interval->time;
+ /* overflow check copied from int4um */
+ if (interval->time != 0 && SAMESIGN(result->time, interval->time))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
result->day = -interval->day;
+ if (interval->day != 0 && SAMESIGN(result->day, interval->day))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
result->month = -interval->month;
+ if (interval->month != 0 && SAMESIGN(result->month, interval->month))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
PG_RETURN_INTERVAL_P(result);
}
*************** interval_pl(PG_FUNCTION_ARGS)
*** 2872,2879 ****
--- 2894,2919 ----
result = (Interval *) palloc(sizeof(Interval));
result->month = span1->month + span2->month;
+ /* overflow check copied from int4pl */
+ if (SAMESIGN(span1->month, span2->month) &&
+ !SAMESIGN(result->month, span1->month))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
+
result->day = span1->day + span2->day;
+ if (SAMESIGN(span1->day, span2->day) &&
+ !SAMESIGN(result->day, span1->day))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
+
result->time = span1->time + span2->time;
+ if (SAMESIGN(span1->time, span2->time) &&
+ !SAMESIGN(result->time, span1->time))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
PG_RETURN_INTERVAL_P(result);
}
*************** interval_mi(PG_FUNCTION_ARGS)
*** 2888,2895 ****
--- 2928,2954 ----
result = (Interval *) palloc(sizeof(Interval));
result->month = span1->month - span2->month;
+ /* overflow check copied from int4mi */
+ if (!SAMESIGN(span1->month, span2->month) &&
+ !SAMESIGN(result->month, span1->month))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
+
result->day = span1->day - span2->day;
+ if (!SAMESIGN(span1->day, span2->day) &&
+ !SAMESIGN(result->day, span1->day))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
+
result->time = span1->time - span2->time;
+ if (!SAMESIGN(span1->time, span2->time) &&
+ !SAMESIGN(result->time, span1->time))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range")));
+
PG_RETURN_INTERVAL_P(result);
}
*************** interval_mul(PG_FUNCTION_ARGS)
*** 2906,2920 ****
Interval *span = PG_GETARG_INTERVAL_P(0);
float8 factor = PG_GETARG_FLOAT8(1);
double month_remainder_days,
! sec_remainder;
int32 orig_month = span->month,
orig_day = span->day;
Interval *result;
result = (Interval *) palloc(sizeof(Interval));
! result->month = (int32) (span->month * factor);
! result->day = (int32) (span->day * factor);
/*
* The above correctly handles the whole-number part of the month and day
--- 2965,2991 ----
Interval *span = PG_GETARG_INTERVAL_P(0);
float8 factor = PG_GETARG_FLOAT8(1);
double month_remainder_days,
! sec_remainder,
! result_double;
int32 orig_month = span->month,
orig_day = span->day;
Interval *result;
result = (Interval *) palloc(sizeof(Interval));
! result_double = span->month * factor;
! if (result_double > INT_MAX || result_double < INT_MIN)
! ereport(ERROR,
! (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
! errmsg("interval out of range")));
! result->month = (int32) result_double;
!
! result_double = span->day * factor;
! if (result_double > INT_MAX || result_double < INT_MIN)
! ereport(ERROR,
! (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
! errmsg("interval out of range")));
! result->day = (int32) result_double;
/*
* The above correctly handles the whole-number part of the month and day
*************** interval_mul(PG_FUNCTION_ARGS)
*** 2954,2960 ****
/* cascade units down */
result->day += (int32) month_remainder_days;
#ifdef HAVE_INT64_TIMESTAMP
! result->time = rint(span->time * factor + sec_remainder * USECS_PER_SEC);
#else
result->time = span->time * factor + sec_remainder;
#endif
--- 3025,3036 ----
/* cascade units down */
result->day += (int32) month_remainder_days;
#ifdef HAVE_INT64_TIMESTAMP
! result_double = rint(span->time * factor + sec_remainder * USECS_PER_SEC);
! if (result_double > INT64_MAX || result_double < INT64_MIN)
! ereport(ERROR,
! (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
! errmsg("interval out of range")));
! result->time = (int64) result_double;
#else
result->time = span->time * factor + sec_remainder;
#endif
diff --git a/src/interfaces/ecpg/pgtypeslib/interval.c b/src/interfaces/ecpg/pgtypeslib/interval.c
new file mode 100644
index efa775d..d603fd4
*** a/src/interfaces/ecpg/pgtypeslib/interval.c
--- b/src/interfaces/ecpg/pgtypeslib/interval.c
*************** recalc:
*** 1036,1041 ****
--- 1036,1043 ----
static int
tm2interval(struct tm * tm, fsec_t fsec, interval * span)
{
+ if ((double)tm->tm_year * MONTHS_PER_YEAR + tm->tm_mon > INT_MAX)
+ return -1;
span->month = tm->tm_year * MONTHS_PER_YEAR + tm->tm_mon;
#ifdef HAVE_INT64_TIMESTAMP
span->time = (((((((tm->tm_mday * INT64CONST(24)) +
On Mon, Jan 27, 2014 at 04:47:22PM -0500, Bruce Momjian wrote:
The updated attached patch has overflow detection for interval
subtraction, multiply, and negate. There are also some comparison
cleanups.
Oh, one odd thing about this patch. I found I needed to use INT64_MAX,
but I don't see it used anywhere else in our codebase. Is this OK? Is
there a better way?
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Bruce Momjian <bruce@momjian.us> writes:
Oh, one odd thing about this patch. I found I needed to use INT64_MAX,
but I don't see it used anywhere else in our codebase. Is this OK? Is
there a better way?
Most of the overflow tests in int.c and int8.c are coded to avoid relying
on the MIN or MAX constants; which seemed like better style at the time.
I'm not sure whether relying on INT64_MAX to exist is portable.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Jan 27, 2014 at 07:19:21PM -0500, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
Oh, one odd thing about this patch. I found I needed to use INT64_MAX,
but I don't see it used anywhere else in our codebase. Is this OK? Is
there a better way?Most of the overflow tests in int.c and int8.c are coded to avoid relying
on the MIN or MAX constants; which seemed like better style at the time.
Yes, I looked at those but they seemed like overkill for interval. For
a case where there was an int64 multiplied by a double, then cast back
to an int64, I checked the double against INT64_MAX before casting to an
int64.
I'm not sure whether relying on INT64_MAX to exist is portable.
The only use I found was in pgbench:
#ifndef INT64_MAX
#define INT64_MAX INT64CONST(0x7FFFFFFFFFFFFFFF)
#endif
so I have just added that to my patch, and INT64_MIN:
#ifndef INT64_MIN
#define INT64_MIN (-INT64CONST(0x7FFFFFFFFFFFFFFF) - 1)
#endif
This is only used for HAVE_INT64_TIMESTAMP.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Jan 27, 2014 at 10:48:16PM -0500, Bruce Momjian wrote:
On Mon, Jan 27, 2014 at 07:19:21PM -0500, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
Oh, one odd thing about this patch. I found I needed to use INT64_MAX,
but I don't see it used anywhere else in our codebase. Is this OK? Is
there a better way?Most of the overflow tests in int.c and int8.c are coded to avoid relying
on the MIN or MAX constants; which seemed like better style at the time.Yes, I looked at those but they seemed like overkill for interval. For
a case where there was an int64 multiplied by a double, then cast back
to an int64, I checked the double against INT64_MAX before casting to an
int64.I'm not sure whether relying on INT64_MAX to exist is portable.
The only use I found was in pgbench:
#ifndef INT64_MAX
#define INT64_MAX INT64CONST(0x7FFFFFFFFFFFFFFF)
#endifso I have just added that to my patch, and INT64_MIN:
#ifndef INT64_MIN
#define INT64_MIN (-INT64CONST(0x7FFFFFFFFFFFFFFF) - 1)
#endifThis is only used for HAVE_INT64_TIMESTAMP.
Adjusted patch applied for PG 9.4. Thanks for the report.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers