Re: New version of money type

On Thu, 21 Dec 2006 10:47:52 -0500
Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "D'Arcy J.M. Cain" <darcy@druid.net> writes:
> > Very good points.  However, like the currency symbol issue I would like
> > to separate that into another discussion.  The code already exists with
> > the warts you mention (and more) and this proposal is to fix one thing
> > that will make it more useful to others.  Let's get that change in and
> > then start fixing up some of those other issues.
> 
> I've forgotten now --- was this patch intended *only* to convert money
> from int4 to int8 underlying representation, or did you do other things?

Well, the main intention was to just widen the underlying storage and
thus increase the range to the point that the type is useful to more
users.  In fact, as you can see, I have removed the change to drop the
currency on output just to keep this change to a single issue.
However, there was a little bit of cleanup as well.  I removed some
self-balancing XXX comments for example.  That's what CVS log is for.
I moved a few functions around in order to make static functions self
prototyping.  I added some consts to variables where appropriate.  The
cash_words function needed to be changed to accomodate the billions,
trillions and quadrillions that can now be handled.

Everything else should be directly related to the type change and
self-explanatory.

> It looks like there are unrelated changes in the patch, but I'm not sure
> if you just moved code around or did something more interesting.

Hopefully nothing too "interesting."  :-)

> One bug I see in it is that you'd better make the alignment 'd' if the

Fixed in my local tree.  Thanks.

> type is to be int8.  Also I much dislike these changes:
> 
> -    int32        i = PG_GETARG_INT32(1);
> +    int64        i = PG_GETARG_INT32(1);
> 
> I think they may not actually be wrong, but they certainly *look* wrong;
> in general the declared type of a parameter variable in a C-coded SQL
> function ought to match what the SQL signature says.  Anyway there is no
> need that I can see to widen these variables.  Every C compiler knows
> what to do if you ask it for arithmetic on a long and an int.

Right but I still need to accept int64 args here.  I have changed the
two relevant places to use PG_GETARG_INT64(1).

> (Speaking of which, have you thought about what happens on a machine
> with no 64-bit int, such that "int64" is really just 32 bits?  Ideally
> the code should continue to function but with reduced range.  I didn't
> see any places where you were obviously depending on the range, but
> it's something to have in the back of your mind while coding.)

Does PGSQL run on any such machines?  If so perhaps someone can
volunteer to do some testing if they have one.

-- 
D'Arcy J.M. Cain <darcy@druid.net>         |  Democracy is three wolves
http://www.druid.net/darcy/                |  and a sheep voting on
+1 416 425 1212     (DoD#0082)    (eNTP)   |  what's for dinner.