Re: Patch for SQL-standard negative valued year-month literals

From: Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Patch for SQL-standard negative valued year-month literals
Date: 2008-09-16 22:20:02
Message-ID: 48D03112.6020705@cheapcomplexdevices.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Tom Lane wrote:
> Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Stark 2008-09-16 22:23:38 Re: Common Table Expressions (WITH RECURSIVE) patch
Previous Message Tom Lane 2008-09-16 21:52:45 Re: Patch for SQL-standard negative valued year-month literals