Ron Mayer <rm_pg@cheapcomplexdevices.com> writes:
> Short summary:
> I think this patch fixes a bug with sql-spec negative interval literals.
Hmm. I'm a bit concerned about possible side-effects on other cases:
what had been seen as two separate tokens will now become one token
for *all* datetime types, not just interval. However, I can't
immediately think of any likely input formats where this would be a
problem.
Something else I noticed while poking at it is this inconsistency:
regression=# select interval '1';interval
----------00:00:01
(1 row)
regression=# select interval '-1';interval
------------01:00:00
(1 row)
regression=# select interval '+1';interval
----------01:00:00
(1 row)
regression=# select interval '1' day;interval
----------1 day
(1 row)
regression=# select interval '+1' day;interval
----------00:00:00
(1 row)
regression=# select interval '-1' day;interval
----------00:00:00
(1 row)
regression=# select interval '1' hour to minute;interval
----------00:01:00
(1 row)
regression=# select interval '-1' hour to minute;interval
------------01:00:00
(1 row)
regression=# select interval '+1' hour to minute;interval
----------01:00:00
(1 row)
As soon as you throw in a sign, it gets wacky :-(.
The reason for this bizarreness is this chunk of code at the end of
DecodeInterval's DTK_TZ case:
else if (type == IGNORE_DTF) { if (*cp == '.') {
/* * Got a decimal point? Then assume some sort of *
secondsspecification */ type = DTK_SECOND; }
else if (*cp == '\0') { /* * Only a signed integer?
Thenmust assume a * timezone-like usage */ type =
DTK_HOUR; } }
which means that a signed integer gets forced to be "hours" if there
isn't an explicit unit spec in the literal, while a signed float gets
forced to be "seconds". I can't see any reason why that's a good idea,
and propose that while we're making incompatible changes in corner cases,
we should just drop the code quoted above.
regards, tom lane
