Re: INTERVAL SECOND limited to 59 seconds?

Hello,

Can someone explain this:

test1=> create table t1 ( k int, i interval second );
CREATE TABLE
test1=> insert into t1 values ( 1, '-67 seconds' );
INSERT 0 1
test1=> insert into t1 values ( 2, '999 seconds' );
INSERT 0 1
test1=> select * from t1;
k | i
---+-----------
1 | -00:00:07
2 | 00:00:39
(2 rows)

I would expect that an INTERVAL SECOND can store more that 59 seconds.

Same question for INTERVAL MINUTE TO SECOND (but here we get an overflow error):

test1=> create table t2 ( k int, i interval minute to second );
CREATE TABLE
test1=> insert into t2 values ( 2, '9999:59' );
ERROR: interval field value out of range: "9999:59"
LINE 1: insert into t2 values ( 2, '9999:59' );
^
test1=> insert into t2 values ( 2, '999:59' );
ERROR: interval field value out of range: "999:59"
LINE 1: insert into t2 values ( 2, '999:59' );
^
test1=> insert into t2 values ( 2, '99:59' );
ERROR: interval field value out of range: "99:59"
LINE 1: insert into t2 values ( 2, '99:59' );
^
test1=> insert into t2 values ( 1, '59:59' );
INSERT 0 1

test1=> insert into t2 values ( 2, '-123:59' );
INSERT 0 1

test1=> select * from t2;
k | i
---+-----------
1 | 00:59:59
2 | -00:59:00
(2 rows)

It's ok when using DAYs:

test1=> create table t3 ( k int, i interval day to second );
CREATE TABLE
test1=> insert into t3 values ( 1, '-9999 18:59:59' );
INSERT 0 1
test1=> insert into t3 values ( 1, '9999999 18:59:59' );
INSERT 0 1
test1=> select * from t3;
k | i
---+-----------------------
1 | -9999 days +18:59:59
1 | 9999999 days 18:59:59
(2 rows)

Thanks a lot!
Seb

Sebastien FLAESCH wrote:
> Hello,
>
> Can someone explain this:
>
> test1=> create table t1 ( k int, i interval second );
> CREATE TABLE
> test1=> insert into t1 values ( 1, '-67 seconds' );
> INSERT 0 1
> test1=> insert into t1 values ( 2, '999 seconds' );
> INSERT 0 1
> test1=> select * from t1;
> k | i
> ---+-----------
> 1 | -00:00:07
> 2 | 00:00:39
> (2 rows)
>
> I would expect that an INTERVAL SECOND can store more that 59 seconds.

I didn't even know we had an "interval second" type. It's not entirely
clear to me what such a value means. Anyway - what's happening is that
it's going through "interval" first. So - '180 seconds' will yield
'00:03:00' and the seconds part of that is zero.

The question I suppose is whether that's correct or not. An interval can
clearly store periods longer than 59 seconds. It's reasonable to ask for
an interval to be displayed as "61 seconds". If "interval second" means
the seconds-only part of an interval though, then it's doing the right
thing.

--
Richard Huxton
Archonet Ltd

I think it should be clarified in the documentation...

Actually I would like to use this new INTERVAL type to store IBM/Informix INTERVALs,
which can actually be used like this with DATETIME types:

> create table t1 (
> k int,
> dt1 datetime hour to minute,
> dt2 datetime hour to minute,
> i interval hour(5) to minute );
Table created.

> insert into t1 values ( 1, '14:45', '05:10', '-145:10' );
1 row(s) inserted.

> select dt1 - dt2 from t1;
(expression)
9:35 <- INTERVAL expression
1 row(s) retrieved.

> select 15 * ( dt1 - dt2 ) from t1;
(expression)
143:45 <- INTERVAL expression
1 row(s) retrieved.

The PostgreSQL documentation says:

The interval type has an additional option, which is to restrict the set of stored
fields by writing one of these phrases:

YEAR
MONTH
DAY
HOUR
MINUTE
SECOND
YEAR TO MONTH
DAY TO HOUR
DAY TO MINUTE
DAY TO SECOND
HOUR TO MINUTE
MINUTE TO SECOND

Does that mean that the [field] option of the INTERVAL type is just there to save
storage space?

Confusing...

Seb

Richard Huxton wrote:
> Sebastien FLAESCH wrote:
>> Hello,
>>
>> Can someone explain this:
>>
>> test1=> create table t1 ( k int, i interval second );
>> CREATE TABLE
>> test1=> insert into t1 values ( 1, '-67 seconds' );
>> INSERT 0 1
>> test1=> insert into t1 values ( 2, '999 seconds' );
>> INSERT 0 1
>> test1=> select * from t1;
>> k | i
>> ---+-----------
>> 1 | -00:00:07
>> 2 | 00:00:39
>> (2 rows)
>>
>> I would expect that an INTERVAL SECOND can store more that 59 seconds.
>
> I didn't even know we had an "interval second" type. It's not entirely
> clear to me what such a value means. Anyway - what's happening is that
> it's going through "interval" first. So - '180 seconds' will yield
> '00:03:00' and the seconds part of that is zero.
>
> The question I suppose is whether that's correct or not. An interval can
> clearly store periods longer than 59 seconds. It's reasonable to ask for
> an interval to be displayed as "61 seconds". If "interval second" means
> the seconds-only part of an interval though, then it's doing the right
> thing.
>

Sebastien FLAESCH wrote:
> I think it should be clarified in the documentation...

Please don't top-quote. And yes, I think you're right.

Hmm a quick google for: [sql "interval second"] suggests that it's not
the right thing. I see some mention of 2 digit precision for a leading
field, but no "clipping".

Looking at the manuals and indeed a quick \dT I don't see "interval
second" listed as a separate type though. A bit of exploring in
pg_attribute with a test table suggests it's just using "interval" with
a type modifier. Which you seem to confirm from the docs:

> The PostgreSQL documentation says:
>
> The interval type has an additional option, which is to restrict the set
> of stored
> fields by writing one of these phrases:
>
> YEAR
> MONTH
...
> Does that mean that the [field] option of the INTERVAL type is just
> there to save
> storage space?

My trusty copy of the 8.3 source suggests that AdjustIntervalForTypmod()
is the function we're interested in and it lives in
backend/utils/adt/timestamp.c - it looks like it just zeroes out the
fields you aren't interested in. No space saving.

So - not a bug, but perhaps not the behaviour you would expect.

> Actually I would like to use this new INTERVAL type to store
> IBM/Informix INTERVALs,
> which can actually be used like this with DATETIME types:
>
> > create table t1 (
> > k int,
> > dt1 datetime hour to minute,
> > dt2 datetime hour to minute,
> > i interval hour(5) to minute );
> Table created.
>
> > insert into t1 values ( 1, '14:45', '05:10', '-145:10' );
> 1 row(s) inserted.
>
> > select dt1 - dt2 from t1;
> (expression)
> 9:35 <- INTERVAL expression

SELECT ('14:45'::time - '05:10'::time);
?column?
----------
09:35:00
(1 row)

> > select 15 * ( dt1 - dt2 ) from t1;
> (expression)
> 143:45 <- INTERVAL expressio

=> SELECT 15 * ('14:45'::time - '05:10'::time);
?column?
-----------
143:45:00
(1 row)

If you can live with the zero seconds appearing, it should all just
work*. Other than formatting as text, I don't know of a way to suppress
them though.

* Depending on whether you need to round up if you ever get odd seconds etc.

--
Richard Huxton
Archonet Ltd

Actually it's not limited to the usage of INTERVAL SECOND, I am writing
a PostgreSQL driver for our 4GL virtual machine...

I need to store all possible Informix INTERVAL types such as:

INTERVAL MONTH(8) TO MONTH
INTERVAL DAY(8) TO MINUTE
INTERVAL SECOND TO FRACTION(5)
... etc ...

...

If PostgreSQL is not able to store months > 11, hours > 23 and minutes
or seconds > 59, it looks like I will need to deal with PostgreSQL's

INTERVAL YEAR TO MONTH
INTERVAL DAY TO SECOND(5)

... and make conversions, to store all possible Informix INTERVALs...

Seb

Sebastien FLAESCH wrote:
> Actually it's not limited to the usage of INTERVAL SECOND, I am writing
> a PostgreSQL driver for our 4GL virtual machine...
>
> I need to store all possible Informix INTERVAL types such as:
>
> INTERVAL MONTH(8) TO MONTH
> INTERVAL DAY(8) TO MINUTE
> INTERVAL SECOND TO FRACTION(5)
> ... etc ...
>
> ...
>
> If PostgreSQL is not able to store months > 11, hours > 23 and minutes
> or seconds > 59

Well, it's not storage it's formatting. Doesn't make any difference to
your problem though.

>, it looks like I will need to deal with PostgreSQL's
>
> INTERVAL YEAR TO MONTH
> INTERVAL DAY TO SECOND(5)
>
> ... and make conversions, to store all possible Informix INTERVALs...

If you know a little "C" you could build some custom types to match your
needs. It should just be a matter of applying the correct formatting as
a wrapper around the existing "interval" type.

--
Richard Huxton
Archonet Ltd

Sebastien FLAESCH wrote:
> Actually it's not limited to the usage of INTERVAL SECOND, I am writing
> a PostgreSQL driver for our 4GL virtual machine...
>
> I need to store all possible Informix INTERVAL types such as:
>
> INTERVAL MONTH(8) TO MONTH
> INTERVAL DAY(8) TO MINUTE
> INTERVAL SECOND TO FRACTION(5)

In Postgres, you should just store it as an INTERVAL which (unlike some
other RDBMS') has the ability to store ranges from fractional seconds to
thousands of years. Then if you need to output it in the above format,
make a view that splits the actual interval into month, minute and
fractional second pieces.

Sebastien FLAESCH <sf(at)4js(dot)com> writes:
> I would expect that an INTERVAL SECOND can store more that 59 seconds.

I took a look into the SQL spec and I think that we do indeed have a
spec compliance issue here. SQL99 section 4.7 saith

Within a value of type interval, the first field is constrained
only by the <interval leading field precision> of the associated
<interval qualifier>. Table 8, "Valid values for fields in INTERVAL
values", specifies the constraints on subsequent field values.
[ Table 8 says about what you'd expect, eg 0..23 for HOUR ]
Values in interval fields other than SECOND are integers and have
precision 2 when not the first field. SECOND, however, can be
defined to have an <interval fractional seconds precision> that
indicates the number of decimal digits maintained following the
decimal point in the seconds value. When not the first field,
SECOND has a precision of 2 places before the decimal point.

So in other words, "999 seconds" is a valid value for a field of type
INTERVAL SECOND, *and should come out the same way*, not as "00:16:39",
and certainly not as "00:00:39".

It might be a relatively easy fix to not truncate the input value
incorrectly. I haven't looked, but I think we should look now, because
8.4 has already changed the behavior in this area and it would be good
not to change it twice. The focus of the 8.4 work was to make sure that
we would correctly interpret the values of spec-compliant interval
literals, but this example shows we are not there yet.

We are fairly far away from being able to make it print out as the spec
would suggest, because interval_out simply doesn't have access to the
information that the field is constrained to be INTERVAL SECOND rather
than some other kind of interval. We also have got no concept at all of
<interval leading field precision>, only of <interval fractional seconds
precision>, so constraining the leading field to only a certain number
of integral digits isn't possible either. I don't foresee anything
getting done about either of those points for 8.4.

regards, tom lane

Thank you Tom for looking at this.

I would be pleased to help on testing the fix when available.

My plan is to store Informix INTERVALs (coming from the 4gl applications we
support) into PostgreSQL INTERVALs, and I have a bunch of tests for that...

I believe Informix INTERVALs (and related operators and functions) are not
100% SQL99, but they are close...

Thanks a lot!
Seb

Tom Lane wrote:
> Sebastien FLAESCH <sf(at)4js(dot)com> writes:
>> I would expect that an INTERVAL SECOND can store more that 59 seconds.
>
> I took a look into the SQL spec and I think that we do indeed have a
> spec compliance issue here. SQL99 section 4.7 saith
>
> Within a value of type interval, the first field is constrained
> only by the <interval leading field precision> of the associated
> <interval qualifier>. Table 8, "Valid values for fields in INTERVAL
> values", specifies the constraints on subsequent field values.
> [ Table 8 says about what you'd expect, eg 0..23 for HOUR ]
> Values in interval fields other than SECOND are integers and have
> precision 2 when not the first field. SECOND, however, can be
> defined to have an <interval fractional seconds precision> that
> indicates the number of decimal digits maintained following the
> decimal point in the seconds value. When not the first field,
> SECOND has a precision of 2 places before the decimal point.
>
> So in other words, "999 seconds" is a valid value for a field of type
> INTERVAL SECOND, *and should come out the same way*, not as "00:16:39",
> and certainly not as "00:00:39".
>
> It might be a relatively easy fix to not truncate the input value
> incorrectly. I haven't looked, but I think we should look now, because
> 8.4 has already changed the behavior in this area and it would be good
> not to change it twice. The focus of the 8.4 work was to make sure that
> we would correctly interpret the values of spec-compliant interval
> literals, but this example shows we are not there yet.
>
> We are fairly far away from being able to make it print out as the spec
> would suggest, because interval_out simply doesn't have access to the
> information that the field is constrained to be INTERVAL SECOND rather
> than some other kind of interval. We also have got no concept at all of
> <interval leading field precision>, only of <interval fractional seconds
> precision>, so constraining the leading field to only a certain number
> of integral digits isn't possible either. I don't foresee anything
> getting done about either of those points for 8.4.
>
> regards, tom lane
>

Finally got around to looking at this thread.
Looks like the original questions from the thread
got resolved, but I found this behaviour surprising:

regression=# select interval '1' day to second;
interval
----------
@ 1 hour
(1 row)

Should this be 1 second?
If so I can send a patch.

regression=# select version();
version
-----------------------------------------------------------------------------------------------------------
PostgreSQL 8.4beta2 on i686-pc-linux-gnu, compiled by GCC gcc (GCC) 4.2.4 (Ubuntu 4.2.4-1ubuntu3), 32-bit
(1 row)

Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com> writes:
> Looks like the original questions from the thread
> got resolved, but I found this behaviour surprising:

> regression=# select interval '1' day to second;
> interval
> ----------
> @ 1 hour
> (1 row)

> Should this be 1 second?

That is a bit odd, especially seeing that eg. '1' hour to second
comes out as 1 second. What's making it do that?

regards, tom lane

This is of course unexpected to me (one day becomes an hour)...

Actually I would even expect an error, because there are missing interval parts.

To represent a valid day to second interval, you should write '1 00:00:00' ...

'1' would be a valid day to day interval.

Always providing all interval units would clarify the user code (avoid complex
rules to get defaults), IMHO.

Just to compare with IFX interval literals:

==============================================================================

> select interval( 1, day to second ) from systables where tabid=1;

201: A syntax error has occurred.
Error in line 1
Near character position 37

> select interval( 1 ) day to second from systables where tabid=1;

1262: Non-numeric character in datetime or interval.
Error in line 1
Near character position 36

> select interval ( 1 11:22:33 ) day to second from systables where tabid=1;

(constant)

1 11:22:33

1 row(s) retrieved.

> select interval ( 1 ) day to day from systables where tabid=1;

(constant)

1

1 row(s) retrieved.

==============================================================================

Seb

Ron Mayer wrote:
> Finally got around to looking at this thread.
> Looks like the original questions from the thread
> got resolved, but I found this behaviour surprising:
>
> regression=# select interval '1' day to second;
> interval
> ----------
> @ 1 hour
> (1 row)
>
> Should this be 1 second?
> If so I can send a patch.
>
>
>
>
>
> regression=# select version();
> version
> -----------------------------------------------------------------------------------------------------------
> PostgreSQL 8.4beta2 on i686-pc-linux-gnu, compiled by GCC gcc (GCC) 4.2.4 (Ubuntu 4.2.4-1ubuntu3), 32-bit
> (1 row)
>

Tom Lane wrote:
> Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com> writes:
>> Looks like the original questions from the thread
>> got resolved, but I found this behaviour surprising:
>
>> regression=# select interval '1' day to second;
>> interval
>> ----------
>> @ 1 hour
>> (1 row)
>
>> Should this be 1 second?
>
> That is a bit odd, especially seeing that eg. '1' hour to second
> comes out as 1 second. What's making it do that?

What from a design point of view? Seems like it's a side
effect of the logic that makes:
select interval '1 2';
know that the 2 means hours rather than seconds.

Code-wise, it seems because around line 2906 in DecodeInterval:
switch (range) ...
case INTERVAL_MASK(DAY) | INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE) | INTERVAL_MASK(SECOND):
type=DTK_HOUR;
But if I naively change that by making it DTK_SECOND,
I'd break "select interval '1 2' day to second;". I guess I'd need
to tweak it to say: if it follows a days filed it means hours; but
by itself it means seconds?

There's a bit of other odd stuff around there. It seems CVS head accepts
"select interval '1 2' hour;" but not "select interval '1 2' hour to minute;"
regression=# select interval '1 2' hour;
interval
----------------
1 day 02:00:00
(1 row)
and I would have guessed that either both should succeed or both should fail.
And if both succeed I wouldn't have expected 1 day 2 hours......

I'd still be happy to send a patch, but am still trying to figure out
what the desired behavior is. My current impression:

What's the desired behavior for each of these:

select interval '1' day to second;
--- should it be 1 second to be consistent with "select interval 1;"?
--- or an error as Sebastien argued in a different part of the thread?

select interval '1 2' hour;
--- should be an error as "select interval '1 2' hour to minute" is?
--- should be "1 day 2 hours" as cvs head treats
"select interval '1 day 2 hours' hour to minute;"?
--- should be 2 hours?

select interval '1 2' hour to minute;
--- should be an error as "select interval '1 2' hour to minute" is?
--- should be "1 day 2 hours" as cvs head treats
"select interval '1 day 2 hours' hour to minute;"?
--- should be 2 hours?

Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com> writes:
> Tom Lane wrote:
>> That is a bit odd, especially seeing that eg. '1' hour to second
>> comes out as 1 second. What's making it do that?

> Code-wise, it seems because around line 2906 in DecodeInterval:
> switch (range) ...
> case INTERVAL_MASK(DAY) | INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE) | INTERVAL_MASK(SECOND):
> type=DTK_HOUR;
> But if I naively change that by making it DTK_SECOND,
> I'd break "select interval '1 2' day to second;". I guess I'd need
> to tweak it to say: if it follows a days filed it means hours; but
> by itself it means seconds?

Well, remember that that code is dealing with an unlabeled rightmost
field. In all the cases except DAY TO MINUTE and DAY TO SECOND, the
choice is to assume that such a field corresponds to the rightmost field
of the declared interval type. So the question is do we want the
current behavior, or do we want to rearrange the switch() so that these
two cases assume MINUTE and SECOND respectively? It's certainly a
trivial code change, but it's not clear what's the right thing.

The cases of interest seem to be outside the spec, so it's not much
help. It says that

22) Let N be the number of <primary datetime field>s in the
precision of the <interval literal>, as specified by <interval
qualifier>.

The <interval literal> being defined shall contain N datetime
components.

and the syntaxes for multi-component literals are mostly not very
ambiguous --- the rightmost component is generally supposed to have
punctuation that disambiguates what it is.

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. It's a judgement
call, but I can't offhand see a case where the current behavior wouldn't
seem surprising.

Plan C would be to say that these cases are ambiguous and we should
throw error. I'm not too thrilled with that, though, especially since
we didn't throw error in prior versions.

regards, tom lane

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

Tom Lane wrote:
> 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.
> ...
> 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.

Seems to do a sane thing for all sane inputs I threw at it.

It still accepts one odd input that 8.3 rejected:
regression=# select interval '1 1' hour;
Perhaps the additional patch below fixes that?

***************
*** 3022,3028 **** DecodeInterval(char **field, int *ftype, int nf, int range,
tm->tm_hour += val;
AdjustFractSeconds(fval, tm, fsec, SECS_PER_HOUR);
tmask = DTK_M(HOUR);
! type = DTK_DAY; /* set for next field */
break;

case DTK_DAY:
--- 3022,3029 ----
tm->tm_hour += val;
AdjustFractSeconds(fval, tm, fsec, SECS_PER_HOUR);
tmask = DTK_M(HOUR);
! if (range == (INTERVAL_MASK(DAY) | INTERVAL_MASK(HOUR)))
! type = DTK_DAY; /* set for next field */
break;

case DTK_DAY:

It also gives different answers than 8.3 for "select interval '1 1:' hour"
but I guess that's intended, right?

Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com> writes:
> It still accepts one odd input that 8.3 rejected:
> regression=# select interval '1 1' hour;
> Perhaps the additional patch below fixes that?

Hmm, not sure about that one. We decided a week or two back that we
don't want the thing discarding higher-order field values, and this
seems pretty close to that. As the code is set up (plus my patch)
I think it's the case that only the rightmost field specification
of the interval qualifier makes any difference for parsing the value;
the leftmost field doesn't really affect what we think the constant
means. That seems like a nice simple consistency property ...

regards, tom lane

Tom Lane wrote:
> Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com> writes:
>> regression=# select interval '1 1' hour;
>
> Hmm, not sure about that one. We decided a week or two back that we
> don't want the thing discarding higher-order field values, and this
> seems pretty close to that. As the code is set up (plus my patch)
> I think it's the case that only the rightmost field specification
> of the interval qualifier makes any difference for parsing the value;
> the leftmost field doesn't really affect what we think the constant
> means. That seems like a nice simple consistency property ...

Sounds good. I'm happy with it then.