Re: BUG #1993: Adding/subtracting negative time intervals

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: Michael Fuhr <mike(at)fuhr(dot)org>, Michael Glaesemann <grzm(at)myrealbox(dot)com>, Klint Gore <kg(at)kgb(dot)une(dot)edu(dot)au>, Russell Smith <mr-russ(at)pws(dot)com(dot)au>, pgsql-bugs(at)postgresql(dot)org, Nicholas <hb(at)x256(dot)com>
Subject: Re: BUG #1993: Adding/subtracting negative time intervals
Date: 2005-10-25 17:28:00
Message-ID: 18319.1130261280@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Tom Lane wrote:
>> Right. Interval multiplication has always spilled fractional months
>> over to seconds, but never the reverse. We have to have that same
>> policy now for fractional days.

> OK, I think that makes sense.

I've applied this change to interval_mul and interval_div, but the
justify_hours call is still there in timestamp_mi. Taking that one out
causes quite a lot of changes in the regression test outputs, so I'm
a bit hesitant to do it. Arguably, we need separate versions of
timestamp_mi and timestamptz_mi, with a DST-aware calculation in the
latter, but that seems a bit large of a change for late beta. The
reason is that with 8.1, we have this discrepancy:

regression=# select '2005-10-29 13:22:00-04'::timestamptz + '1 day'::interval;
?column?
------------------------
2005-10-30 13:22:00-05
(1 row)

regression=# select '2005-10-30 13:22:00-05'::timestamptz - '2005-10-29 13:22:00-04'::timestamptz;
?column?
----------------
1 day 01:00:00
(1 row)

ISTM that given the former result, the latter calculation ought to
produce '1 day', not something else.

Another problem I've noticed is that interval output works with a
"struct tm" as intermediate data structure, which means that it cannot
cope with intervals containing a "time" field exceeding 2^31 hours,
because the tm_hour field overflows. With the new version of
interval_mul this is easily exposed by this test case:

regression=# select 10000 * '1000000 hours'::interval;
?column?
------------------
2147483647:00:00
(1 row)

but it was possible to get the same problem in other ways before,
so I don't think this is interval_mul's fault. Rather, interval2tm
has got to be replaced with something that can handle the full range of
representable interval values.

Finally, I notice there are no overflow checks in any of the interval
or timestamp arithmetic routines. This seems like a bad omission,
particularly in the integer-timestamp case where overflow won't be even
a little bit graceful.

So, a few TODO items for future releases:

* Improve timestamptz subtraction to be DST-aware
* Fix interval display to support values exceeding 2^31 hours
* Add overflow checking to timestamp and interval arithmetic

regards, tom lane

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Tom Lane 2005-10-25 18:16:08 Re: RIGHT JOIN is only supported with merge-joinable join conditions, PostgreSQL 8.1 beta3
Previous Message Jean-Pierre Pelletier 2005-10-25 16:02:34 RIGHT JOIN is only supported with merge-joinable join conditions, PostgreSQL 8.1 beta3

Browse pgsql-hackers by date

  From Date Subject
Next Message Josh Berkus 2005-10-25 18:08:08 Re: determining random_page_cost value
Previous Message Yohanes Santoso 2005-10-25 16:29:21 determining random_page_cost value