Tom Lane wrote:
> 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
If it's a concern, I could make interval_in first look for the
SQL-standard patterns before even parsing the string into fields.
If we want to handle the SQL standard negative datetime intervals
(see below) the way the spec looks to me,
> immediately think of any likely input formats where this would be a
> problem.
> Something else I noticed while poking at it is this inconsistency:...
Yes. I saw some of those too (and '-1 1:00:00'); but didn't have a
patch ready (yet). I'm happy to work on it.
> As soon as you throw in a sign, it gets wacky :-(.
Oh. And looking more closely; there's a potential bad incompatibility.
If I read SQL 200N's spec correctly
select interval '-1 1:00:00';
should mean "-1 days -1 hours",
yet 8.3 sees it as "-1 days +1 hours".
Scary to touch that one, but since a standard's a standard, I think
eventually we should get there. Perhaps we need a GUC to choose
standards or backward compatible behavior for that one? Or perhaps
keep parsing it the old way but with a WARNING for 8.4 and switch in 8.5?
> The reason for this bizarreness is this chunk of code at the end of
> DecodeInterval's DTK_TZ case:
>
> else if (type == IGNORE_DTF)
> {...} }
>
> 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.
+1. Shall I try for a patch that handles those and possibly the (more
frightening) SQL 200N signed day-time interval mentioned above.
