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

On Mon, Jun 1, 2009 at 8:04 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I wrote:
>> Sam Mason <sam(at)samason(dot)me(dot)uk> writes:
>>> On Sun, May 31, 2009 at 06:32:53PM -0400, Tom Lane wrote:
>>>> 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!
>
>> Well, maybe, but I'm not really convinced.
>
> I've gone ahead and committed the patch as-is, without the error tests.
> There's still time to change it if anyone has a killer argument, but
> I thought of another issue that would have to be dealt with: consider
> values such as INTERVAL '13' MONTH.  Since per spec we should not
> reduce this to 1 month, what is going to happen barring significant
> redesign on the output side is that the value will print out as
> '1 year 1 month'.  If we were to consider that as illegal input for
> INTERVAL MONTH then we'd be creating a situation where valid data
> fails to dump and reload.  This won't happen for all cases (eg 60
> days doesn't overflow into months) but it shows the danger of throwing
> error for cases that we can't clearly distinguish on both input and
> output.  So I think we should be satisfied for now with accepting
> inputs that are valid per spec, and not worry too much about whether
> we are rejecting all values that are a bit outside spec.

Well, there is the possibility that if we implement something fully
spec-compliant in the future, we might run into a situation where
someone puts 13 months in, dumps and reloads, then puts in 13 months
in again, compares the two, and surprisingly they turn out to be
unequal. But I'm having a hard time caring. The behavior your patch
implements is clearly a lot more useful than what it replaced, and I
think it's arguably more useful than the spec behavior as well.

More to the point, it's also what 8.3.7 does:

Welcome to psql 8.3.7, the PostgreSQL interactive terminal.

Type: \copyright for distribution terms
\h for help with SQL commands
\? for help with psql commands
\g or terminate with semicolon to execute query
\q to quit

rhaas=# select '99 seconds'::interval;
interval
----------
00:01:39
(1 row)

rhaas=# select '99 minutes'::interval;
interval
----------
01:39:00
(1 row)

rhaas=# select '99 hours'::interval;
interval
----------
99:00:00
(1 row)

rhaas=# select '99 days'::interval;
interval
----------
99 days
(1 row)

rhaas=# select '99 weeks'::interval;
interval
----------
693 days
(1 row)

rhaas=# select '99 months'::interval;
interval
----------------
8 years 3 mons
(1 row)

I haven't checked, but hopefully these all now match the 8.4 behavior?

...Robert