Re: Patch for ISO-8601-Interval Input and output.

From: "Brendan Jurd" <direvus(at)gmail(dot)com>
To: "Ron Mayer" <rm_pg(at)cheapcomplexdevices(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Patch for ISO-8601-Interval Input and output.
Date: 2008-11-07 04:58:48
Message-ID: 37ed240d0811062058n752c3880kb04feb85e355b524@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Nov 7, 2008 at 3:35 AM, Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com> wrote:
> I think I updated the web site and git now, and
> 'P0000-00-01' is now accepted. It might be useful if
> someone double checked my reading of the spec, tho.
>

Hi Ron,

I've tested out your latest revision and read the spec more closely,
and pretty much everything works as expected. 'P0000-00-01' yields 1
day, and various other combinations like 'P0000-01-00' and 'P0000-01'
work correctly too.

I agree with your interpretation of the spec, it clearly says that 'T'
can be omitted when there are no time components. This should apply
equally to both the designators format and the alternative format.
The examples in Annex B confirm this.

I did run into one potential bug:

postgres=# select interval 'P0001';
interval
----------
1 day

Whereas, I expected to get '1 year', since the format allows you to
omit lower-order components from right-to-left:

P0001-01-01 => 1 year 1 month 1 day
P0001-01 => 1 year 1 month
P0001 => should be 1 year?

On the documentation front, I have a few final cleanups to suggest
(patch attached).

* After giving the spec a closer look, I thought that 4.4.3.2 and
4.4.3.3 were the proper spec references to use for the two formats.

* I removed the identation from a <programlisting> element. These
elements are left at the start of the line to prevent the initial
spacing from being interpreted as part of the listing itself.

* I made "Interval Output" a level 3 section again, and this time I
corrected that structural issue in my earlier patch. It's now
contained within the level 2 section.

* There was a sentence in the docs about the 'T' separator being
mandatory in the "alternative format". I deleted it.

* Changed "format with time-unit designators" to just "format with
designators", as that is how the spec refers to it.

* Some tabs had crept into the indentation, and there were a few
indentation errors in the new tables. I corrected those (the tabs may
have been my fault; I sometimes forget to change my vi settings when
switching between C code and SGML).

Cheers,
BJ

Attachment Content-Type Size
iso8601_interval-2.diff.bz2 application/x-bzip2 2.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2008-11-07 05:59:45 Re: auto_explain contrib moudle
Previous Message Robert Haas 2008-11-07 04:06:17 Re: array_length()