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

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?

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.

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?

Cheers,
BJ