Brendan Jurd wrote:
> On Sat, Nov 1, 2008 at 3:42 PM, Ron Mayer <rm_pg@cheapcomplexdevices.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