From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | pgsql-hackers(at)postgreSQL(dot)org |
Cc: | Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com>, Sam Mason <sam(at)samason(dot)me(dot)uk> |
Subject: | Patch: AdjustIntervalForTypmod shouldn't discard high-order data |
Date: | 2009-05-31 22:32:53 |
Message-ID: | 16963.1243809173@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
As I mentioned a bit ago
http://archives.postgresql.org/pgsql-hackers/2009-05/msg01505.php
there seems to be a definite problem still remaining with our handling
of interval literals. To wit, this behavior is absolutely not per spec:
regression=# select '999'::interval second;
interval
----------
00:00:39
(1 row)
The correct interpretation of the input value is certainly 999 seconds.
The spec would allow us to throw error if it exceeds the available range
of the field, but a silent modulo operation is not per spec and seems
against our general design principle of not silently discarding data.
I propose the attached patch to make the code not throw away high-order
values in this fashion.
A somewhat more debatable case is this:
regression=# select '1 day 1 hour'::interval hour;
interval
----------
01:00:00
(1 row)
which with the attached patch we would render as
regression=# select '1 day 1 hour'::interval hour;
interval
----------------
1 day 01:00:00
(1 row)
There is some case to be made that we should throw error here,
which we could do by putting error tests where the attached patch
has comments suggesting an error test. However I'm inclined to think
that doing that would expose an implementation dependency rather more
than we should. It is usually not clear to novices that '1 day 1 hour'
is different from '25 hours', and it would be even less clear why the
latter would be acceptable input for an INTERVAL HOUR field when the
former isn't. So I'm proposing the patch as-is rather than with the
extra error tests, but am open to being convinced otherwise.
The reason I'm bringing this up now is that we've already changed the
behavior of interval literals quite a bit in 8.4. I would rather try to
finish getting it right in this release than have the behavior change
twice in successive releases.
Comments?
regards, tom lane
Attachment | Content-Type | Size |
---|---|---|
unknown_filename | text/plain | 11.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Greg Stark | 2009-05-31 22:47:16 | Re: search_path improvements |
Previous Message | Tom Lane | 2009-05-31 21:35:33 | Re: INTERVAL SECOND limited to 59 seconds? |