Hello, At Tue, 21 Mar 2017 07:52:25 -0700, Adrian Klaver <adrian.klaver@xxxxxxxxxxx> wrote in <375c9e5a-960f-942c-913f-55632a1f0a90@xxxxxxxxxxx> > On 03/21/2017 07:42 AM, Tom Lane wrote: > > Frazer McLean <frazer@xxxxxxxxxxxxxxxxxx> writes: > >> I came across an unexpected comparison (tested on PostgreSQL 9.4 and > >> 9.6) for intervals with a large difference in magnitude. > > > >> '1 year'::interval > '32618665 years'::interval; > > > >> Is this a bug? > > > > It looks like the problem is overflow of the result of > > interval_cmp_value, > > because it's trying to compute > > > > =# select '32618665'::int8 * 30 * 86400 * 1000000; > > ERROR: bigint out of range > > > > It's not immediately obvious how to avoid that while preserving the > > same > > comparison semantics :-( This is an apparent bug of interval comparison. During comparison interval is converted into int64 in milliseconds but it overflows in the case. Detecting the overflow during the conversion can fix it and preserving the semantics (except value range). The current code tells a lie anyway for the cases but I'm not sure limting the range of value is acceptable or not. | =# select '106751990 days 24:59:59'::interval; | interval | ------------------------- | 106751990 days 24:59:59 | =# select '106751990 days 24:59:59'::interval > '1 year'::interval; | ERROR: interval out of range during comparison If this is not acceptable, some refactoring would be required. > Not sure if it helps but this works: > > test=# select extract(epoch from '1 year'::interval) > extract(epoch > from '32618665 years'::interval); > ?column? > ---------- > f It calculates in seconds. So it is useful if subseconds are not significant. But extract also silently overflows during converting the same interval to usecs. This seems to need the same amendment. > =# select extract(usec from '32618665 years'::interval); > date_part > ----------- > 0 regards, -- Kyotaro Horiguchi NTT Open Source Software Center
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c index 4be1999..f77cfcc 100644 --- a/src/backend/utils/adt/timestamp.c +++ b/src/backend/utils/adt/timestamp.c @@ -2293,10 +2293,23 @@ static inline TimeOffset interval_cmp_value(const Interval *interval) { TimeOffset span; + TimeOffset timedayfraction; + TimeOffset timedays; - span = interval->time; - span += interval->month * INT64CONST(30) * USECS_PER_DAY; - span += interval->day * INT64CONST(24) * USECS_PER_HOUR; + timedays = ((int64)interval->time) / USECS_PER_DAY; + timedayfraction = interval->time - timedays * USECS_PER_DAY; + + /* calculate span in days. this cannot overflow */ + span = timedays; + span += interval->month * INT64CONST(30); + span += interval->day; + + /* overflow check */ + if (span > INT64CONST(0x7fffffffffffffff) / USECS_PER_DAY - 1) + ereport(ERROR, + (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE), + errmsg ("interval out of range during comparison"))); + span = span * USECS_PER_DAY + timedayfraction; return span; } @@ -2304,6 +2317,7 @@ interval_cmp_value(const Interval *interval) static int interval_cmp_internal(Interval *interval1, Interval *interval2) { + TimeOffset span1 = interval_cmp_value(interval1); TimeOffset span2 = interval_cmp_value(interval2);
-- Sent via pgsql-general mailing list (pgsql-general@xxxxxxxxxxxxxx) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-general