Re: Patch for ISO-8601-Interval Input and output. - Mailing list pgsql-hackers

From Brendan Jurd
Subject Re: Patch for ISO-8601-Interval Input and output.
Date
Msg-id 37ed240d0811050750v11726fd1of441646f87b583da@mail.gmail.com
Whole thread Raw
In response to Patch for ISO-8601-Interval Input and output.  (Ron Mayer <rm_pg@cheapcomplexdevices.com>)
Responses Re: Patch for ISO-8601-Interval Input and output.
List pgsql-hackers
On Thu, Oct 2, 2008 at 9:31 PM, Ron Mayer <rm_pg@cheapcomplexdevices.com> wrote:
> Ron Mayer wrote:
> This patch (that works on top of the IntervalStyle patch I
> posted earlier today) adds support for ISO8601 standard[0]
> "Time Interval" "Durations" of the "format with designators"
> (section 4.4.4.2.1).   The other ISO 8601 types of intervals
> deal with start and end points, so this one seemed most relevant.
>

Hi Ron,

Reviewing this patch now; I'm working from the 'iso8601' branch in
your git repository.

Compile, regression tests and my own ad hoc tests in psql all worked
without a hitch.

As I was reading through the new documentation introduced by the
patch, I thought of various improvements and decided to try them out
for myself.  In the end, rather than try to explain all of my changes
in detail, I thought I'd post a patch of my own (against your branch)
and accompany it with a few explanatory notes.

The patch includes some minor changes to the code style in
src/backend/utils/adt/datetime.c, similar to my recommendations for
the IntervalStyle patch.

As for the documentation, I ended up making quite a few changes.
Explanations and rationales to follow.

ISO section numbers -- in a couple of places, the documentation
referred to the wrong parts of ISO 8601.  I found references to
5.5.4.2.1 and 5.5.4.2.2, whereas in both cases the first two digits
should be '4'.

Interval Input syntax -- I thought that the paragraph explaining the
ISO input syntax was not verbose enough.  One paragraph didn't seem
sufficient to explain the syntaxes involved.  In particular, the
original paragraph didn't mention that units could be omitted or
reordered in the "designators" format.  I expanded a bit more on how
the syntaxes worked, using the same pseudogrammar style used to
explain the postgres interval format, and included a table showing the
available time-unit designators.

Interval Output section structure -- the new section for Interval
Output was added at level 2, right below Date/Time Output.  I felt
this was uncomfortably assymetric with the Input section, which has
Date/Time Input as a level 2 section, with Interval Input as a level 3
underneath it.  I demoted Interval Output to a level 3 and moved it
underneath Date/Time Output.

The rest of my doc changes were adding extra linkage, and some minor
wording and consistency tweaks.

Most of these changes are of a highly subjective nature.  I think they
are improvements, but someone else might prefer the way it was before
I got my hands dirty.  Please consider the changes in my patch a set
of suggestions for making the documentation on this feature a little
easier to digest.  You're welcome to take or leave them as you see
fit.

Cheers,
BJ

Attachment

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Proposed Patch to Improve Performance of Multi-Batch Hash Join for Skewed Data Sets
Next
From: "Joshua Tolley"
Date:
Subject: Re: Proposed Patch to Improve Performance of Multi-Batch Hash Join for Skewed Data Sets