Re: Interval output bug in HAVE_INT64_TIMESTAMP

From: Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com>
To: Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Interval output bug in HAVE_INT64_TIMESTAMP
Date: 2008-10-02 20:30:21
Message-ID: 48E52F5D.2000005@cheapcomplexdevices.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Ron Mayer wrote:
> Tom Lane wrote:
>> Yeah, bug all the way back --- applied.
>> I don't much like the forced rounding to two digits here, but changing
>> that doesn't seem like material for back-patching. Are you going to
>> fix that up while working on your other patches?
>
> Gladly. I hate that too.

Attached is a WIP interval code-cleanup patch that tries to reuse
more code between interval style. Once side-benefit is that it
makes rounding more consistent (so we should have fewer bugs like
that last rounding one that only affected one date style), as well
as removing roughly 250 lines of near-copy-and-paste code (some
of it my doing in my previous patches) and IMHO making the code
easier to read.

This particular patch lives on top of the other 2 I submitted,
but I could make a similar one that doesn't depend on those
if either of those get rejected - though the savings wouldn't
be as great since some of the benefit is that all 4 datestyles
can share some of the refactored code. It does do some
extra function calls; but I think any cost there's easily
offset by the removal of gratuitous (AFAICT) strlen()'s that
were scattered in the existing code and are now removed.

Despite the rounding differences it seems to have no effect
on the regression test output - presumably because these rounding
differences are more obscure corner cases.

I consider it a WIP for 3 reasons:

1) Does anyone object to the not-quite-backward compatible
rounding (forced 2 digits; now some rounding instead
of truncation).
2) Do we even want broad scale refactoring like this, or do
people prefer minimal changes (like my previous patches)
that tried to as much of the old code intact preferred.
3) If people like this kind of refactoring, there's more
I could do - especially if I can I find a place to put
functions the ECPG stuff can call too.

Attachment Content-Type Size
cleanup.patch text/x-diff 20.6 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Aidan Van Dyk 2008-10-02 21:07:00 Re: Block-level CRC checks
Previous Message Alvaro Herrera 2008-10-02 20:18:12 Re: Block-level CRC checks