Re: Patch: AdjustIntervalForTypmod shouldn't discard high-order data

On Sun, May 31, 2009 at 06:32:53PM -0400, Tom Lane wrote:
> regression=# select '999'::interval second;
> The correct interpretation of the input value is certainly 999 seconds.

Agreed; silent truncation like this is confusing and will lead to
unnecessary bugs in users' code.

> the attached patch we would render as
> regression=# select '1 day 1 hour'::interval hour;
> 1 day 01:00:00
>
> 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.

With things as they are I think it would be useful to throw an error
here; if the user means 25 hours they should say 25 hours!

> 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.

It would only be different when the interval is used with values of type
timestamptz, or am I missing something? How much sense does it make to
have a timezone aware interval where this distinction is true and leave
the current interval as timezone naive. Not sure if that would help to
clean up the semantics at all or if it's just adding more unnecessary
complexity. I have a feeling it's probably the latter, but thought it
may help things.

--
Sam http://samason.me.uk/