Re: interval typmodout is broken

On Thu, Sep 25, 2014 at 12:06:56AM -0400, Tom Lane wrote:
> Alvaro Herrera <alvherre@2ndquadrant.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 "("
> > L�NEA 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@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +