On Thu, Sep 25, 2014 at 12:06:56AM -0400, Tom Lane wrote:
> Alvaro Herrera <[email protected]> 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 <[email protected]> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
new file mode 100644
index c98c27a..0de9584
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
*************** zone_value:
*** 1552,1578 ****
t->typmods = $3;
$$ = makeStringConstCast($2, @2, t);
}
! | ConstInterval '(' Iconst ')' Sconst opt_interval
{
TypeName *t = $1;
! if ($6 != NIL)
! {
! A_Const *n = (A_Const *) linitial($6);
! if ((n->val.val.ival & ~(INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE))) != 0)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("time zone interval must be HOUR or HOUR TO MINUTE"),
! parser_errposition(@6)));
! if (list_length($6) != 1)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("interval precision specified twice"),
! parser_errposition(@1)));
! t->typmods = lappend($6, makeIntConst($3, @3));
! }
! else
! t->typmods = list_make2(makeIntConst(INTERVAL_FULL_RANGE, -1),
! makeIntConst($3, @3));
$$ = makeStringConstCast($5, @5, t);
}
| NumericOnly { $$ = makeAConst($1, @1); }
--- 1552,1562 ----
t->typmods = $3;
$$ = makeStringConstCast($2, @2, t);
}
! | ConstInterval '(' Iconst ')' Sconst
{
TypeName *t = $1;
! t->typmods = list_make2(makeIntConst(INTERVAL_FULL_RANGE, -1),
! makeIntConst($3, @3));
$$ = makeStringConstCast($5, @5, t);
}
| NumericOnly { $$ = makeAConst($1, @1); }
*************** SimpleTypename:
*** 10582,10602 ****
$$ = $1;
$$->typmods = $2;
}
! | ConstInterval '(' Iconst ')' opt_interval
{
$$ = $1;
! if ($5 != NIL)
! {
! if (list_length($5) != 1)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("interval precision specified twice"),
! parser_errposition(@1)));
! $$->typmods = lappend($5, makeIntConst($3, @3));
! }
! else
! $$->typmods = list_make2(makeIntConst(INTERVAL_FULL_RANGE, -1),
! makeIntConst($3, @3));
}
;
--- 10566,10576 ----
$$ = $1;
$$->typmods = $2;
}
! | ConstInterval '(' Iconst ')'
{
$$ = $1;
! $$->typmods = list_make2(makeIntConst(INTERVAL_FULL_RANGE, -1),
! makeIntConst($3, @3));
}
;
*************** AexprConst: Iconst
*** 12923,12943 ****
t->typmods = $3;
$$ = makeStringConstCast($2, @2, t);
}
! | ConstInterval '(' Iconst ')' Sconst opt_interval
{
TypeName *t = $1;
! if ($6 != NIL)
! {
! if (list_length($6) != 1)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("interval precision specified twice"),
! parser_errposition(@1)));
! t->typmods = lappend($6, makeIntConst($3, @3));
! }
! else
! t->typmods = list_make2(makeIntConst(INTERVAL_FULL_RANGE, -1),
! makeIntConst($3, @3));
$$ = makeStringConstCast($5, @5, t);
}
| TRUE_P
--- 12897,12907 ----
t->typmods = $3;
$$ = makeStringConstCast($2, @2, t);
}
! | ConstInterval '(' Iconst ')' Sconst
{
TypeName *t = $1;
! t->typmods = list_make2(makeIntConst(INTERVAL_FULL_RANGE, -1),
! makeIntConst($3, @3));
$$ = makeStringConstCast($5, @5, t);
}
| TRUE_P
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
new file mode 100644
index cf20506..c873a99
*** a/src/test/regress/expected/interval.out
--- b/src/test/regress/expected/interval.out
*************** SELECT interval '12:34.5678' minute to s
*** 616,631 ****
00:12:34.57
(1 row)
- SELECT interval(2) '12:34.5678' minute to second; -- historical PG
- interval
- -------------
- 00:12:34.57
- (1 row)
-
- SELECT interval(2) '12:34.5678' minute to second(2); -- syntax error
- ERROR: interval precision specified twice
- LINE 1: SELECT interval(2) '12:34.5678' minute to second(2);
- ^
SELECT interval '1.234' second;
interval
--------------
--- 616,621 ----
diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql
new file mode 100644
index 2318917..789c3de
*** a/src/test/regress/sql/interval.sql
--- b/src/test/regress/sql/interval.sql
*************** SELECT interval '123 2:03 -2:04'; -- not
*** 183,190 ****
SELECT interval(0) '1 day 01:23:45.6789';
SELECT interval(2) '1 day 01:23:45.6789';
SELECT interval '12:34.5678' minute to second(2); -- per SQL spec
- SELECT interval(2) '12:34.5678' minute to second; -- historical PG
- SELECT interval(2) '12:34.5678' minute to second(2); -- syntax error
SELECT interval '1.234' second;
SELECT interval '1.234' second(2);
SELECT interval '1 2.345' day to second(2);
--- 183,188 ----
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
