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 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"
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 range
>
> SELECT 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 <br...@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
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)) +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
