Re: INTERVAL SECOND limited to 59 seconds?

I wrote:
> I'm inclined to say that these two cases are out of line with what
> the rest of the code does and we should change them.

Specifically, I'm thinking of a patch like this:

Index: datetime.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/datetime.c,v
retrieving revision 1.206
diff -c -r1.206 datetime.c
*** datetime.c 1 Jun 2009 16:55:11 -0000 1.206
--- datetime.c 9 Jun 2009 22:47:50 -0000
***************
*** 2917,2933 ****
break;
case INTERVAL_MASK(HOUR):
case INTERVAL_MASK(DAY) | INTERVAL_MASK(HOUR):
- case INTERVAL_MASK(DAY) | INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE):
- case INTERVAL_MASK(DAY) | INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE) | INTERVAL_MASK(SECOND):
type = DTK_HOUR;
break;
case INTERVAL_MASK(MINUTE):
case INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE):
type = DTK_MINUTE;
break;
case INTERVAL_MASK(SECOND):
- case INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE) | INTERVAL_MASK(SECOND):
case INTERVAL_MASK(MINUTE) | INTERVAL_MASK(SECOND):
type = DTK_SECOND;
break;
default:
--- 2917,2933 ----
break;
case INTERVAL_MASK(HOUR):
case INTERVAL_MASK(DAY) | INTERVAL_MASK(HOUR):
type = DTK_HOUR;
break;
case INTERVAL_MASK(MINUTE):
case INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE):
+ case INTERVAL_MASK(DAY) | INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE):
type = DTK_MINUTE;
break;
case INTERVAL_MASK(SECOND):
case INTERVAL_MASK(MINUTE) | INTERVAL_MASK(SECOND):
+ case INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE) | INTERVAL_MASK(SECOND):
+ case INTERVAL_MASK(DAY) | INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE) | INTERVAL_MASK(SECOND):
type = DTK_SECOND;
break;
default:

Experimenting with this, I confirm that it does these things:

regression=# select '100':: interval day to minute;
interval
----------
01:40:00
(1 row)

regression=# select '100':: interval day to second;
interval
----------
00:01:40
(1 row)

which seem a lot less surprising than the current behavior.
It also causes these changes in the regression tests:

*** src/test/regress/expected/interval.out Mon Jun 1 19:29:37 2009
--- src/test/regress/results/interval.out Tue Jun 9 18:49:32 2009
***************
*** 466,476 ****
(1 row)

SELECT interval '1 2' day to minute;
! interval
! ----------------
! 1 day 02:00:00
! (1 row)
!
SELECT interval '1 2:03' day to minute;
interval
----------------
--- 466,474 ----
(1 row)

SELECT interval '1 2' day to minute;
! ERROR: invalid input syntax for type interval: "1 2"
! LINE 1: SELECT interval '1 2' day to minute;
! ^
SELECT interval '1 2:03' day to minute;
interval
----------------
***************
*** 484,494 ****
(1 row)

SELECT interval '1 2' day to second;
! interval
! ----------------
! 1 day 02:00:00
! (1 row)
!
SELECT interval '1 2:03' day to second;
interval
----------------
--- 482,490 ----
(1 row)

SELECT interval '1 2' day to second;
! ERROR: invalid input syntax for type interval: "1 2"
! LINE 1: SELECT interval '1 2' day to second;
! ^
SELECT interval '1 2:03' day to second;
interval
----------------
***************
*** 605,615 ****
(1 row)

SELECT interval '1 2.345' day to second(2);
! interval
! ----------------
! 1 day 02:20:42
! (1 row)
!
SELECT interval '1 2:03' day to second(2);
interval
----------------
--- 601,609 ----
(1 row)

SELECT interval '1 2.345' day to second(2);
! ERROR: invalid input syntax for type interval: "1 2.345"
! LINE 1: SELECT interval '1 2.345' day to second(2);
! ^
SELECT interval '1 2:03' day to second(2);
interval
----------------

Now, all three of these cases throw "invalid input syntax" in 8.3,
so this is not a regression from released behavior. The question
is does anyone think that these syntaxes should be valid? They're
not legal per spec, for sure, and they seem pretty ambiguous to me.

regards, tom lane