Re: interval typmodout is broken

On Thu, Sep 25, 2014 at 12:06:56AM -0400, Tom Lane wrote:
> Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
> > Tom Lane wrote:
> >> You sure about that? The grammar for INTERVAL is weird.
>
> > Well, I tested what is taken on input, and yes I agree the grammar is
> > weird (but not more weird than timestamp/timestamptz, mind). The input
> > function only accepts the precision just after the INTERVAL keyword, not
> > after the fieldstr:
>
> > alvherre=# create table str (a interval(2) hour to minute);
> > CREATE TABLE
>
> > alvherre=# create table str2 (a interval hour to minute(2));
> > ERROR: syntax error at or near "("
> > LNEA 1: create table str2 (a interval hour to minute(2));
> > ^
>
> No, that's not about where it is, it's about what the field is: only
> "second" can have a precision. Our grammar is actually allowing stuff
> here that it shouldn't. According to the SQL spec, you could write
> interval hour(2) to minute
> but this involves a "leading field precision", which we do not support
> and should definitely not be conflating with trailing-field precision.
> Or you could write
> interval hour to second(2)
> which is valid and we support it. You can *not* write
> interval hour to minute(2)
> either per spec or per our implementation; and
> interval(2) hour to minute
> is 100% invalid per spec, even though our grammar goes out of its
> way to accept it.
>
> In short, the typmodout function is doing what it ought to. It's the
> grammar that's broken. It looks to me like Tom Lockhart coded the
> grammar to accept a bunch of cases that he never got round to actually
> implementing reasonably. In particular, per SQL spec these are
> completely different animals:
> interval hour(2) to second
> interval hour to second(2)
> but our grammar transforms them into the same thing.
>
> We ought to fix that...

I did not find any cases where we support 'INTERVAL HOUR(2) to SECOND'.

I think the basic problem is that the original author had the idea of
doing:

SELECT INTERVAL (2) '100.9999 seconds';
interval
----------
00:01:41

and using (2) in that location as a short-hand when the interval
precision units were not specified, which seems logical. However, they
allowed it even when the units were specified:

SELECT INTERVAL (2) '100.9999 seconds' HOUR to SECOND;
interval
----------
00:01:41

and in cases where the precision made no sense:

SELECT INTERVAL (2) '100.9999 seconds' HOUR to MINUTE;
interval
----------
00:01:00

I have created the attached patch which only allows parentheses in the
first case.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +