Brendan Jurd wrote:
On Sat, Nov 1, 2008 at 3:42 PM, Ron Mayer <[EMAIL PROTECTED]> 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
