Re: Re: Updated interval patches (SQL std output, ISO8601 intervals, and interval rounding)

From: Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com>
To: Brendan Jurd <direvus(at)gmail(dot)com>
Cc: Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Re: Updated interval patches (SQL std output, ISO8601 intervals, and interval rounding)
Date: 2008-11-11 18:32:35
Message-ID: 4919CFC3.3030305@cheapcomplexdevices.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Brendan Jurd wrote:
> On Sat, Nov 1, 2008 at 3:42 PM, Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com> wrote:
>> # Patch 3: "cleanup.patch"
>> Fix rounding inconsistencies and refactor interval input/output code
>
> Compile, testing and regression tests all checked out.
> I've picked up on a few code style issues, fixes attached.
>
> If I'm reading the patch correctly, it seems you've renamed two of the
> functions in datetime.c:
> * AdjustFractionalSeconds => AdjustFractSeconds
> * AdjustFractionalDays => AdjustFractDays
> To be frank, this doesn't really seem worthwhile. It only saves five
> characters in the name. What was your reason for renaming them?

Otherwise many lines were over 80 chars long.
And it happened often enough I thought the shorter name
was less ugly than splitting the arguments in many of the
places where it's called.

I'm happy either way, tho.

> I *was* going to question the inconsistent use of a space between the
> pointer qualifier and the argument name, for example:
>
> static char *
> AddVerboseIntPart(char * cp, int value, char *units,
> bool * is_zero, bool *is_before)
>
> But then I noticed that there's a lot of this going on in datetime.c,
> some of it appears to predate your patches. So I guess cleaning this
> up in your function definitions would be a bit of a bolted-horse,
> barn-door affair. Unless you felt like cleaning it up throughout the
> file, it's probably not worth worrying about.

I don't mindn cleaning it up; but someone could point me to which direction.

> There are some very large-scale changes to the regression tests. I'm
> finding it difficult to grok the nature of the changes from reading a
> diff. If possible, could you post some quick notes on the
> purpose/rationale of these changes?

Previously, much (but IIRC not quite all) of the interval output stuff
rounded to the hundredths place regardless of how many significant digits
there were. So, for example, the interval "1.699" seconds would usually
appear as "1.70" for most but not all combinations of DateStyle
and HAVE_INT64_TIMESTAMP. After a few discussions on the mailing list
I think people decided to simply show the digits that were

http://archives.postgresql.org/pgsql-hackers/2008-09/msg00998.php

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2008-11-11 18:48:59 Re: SQL5 budget
Previous Message Andrew Dunstan 2008-11-11 18:26:38 Re: Question about SPI_prepare