Re: New version of money type

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "D'Arcy J(dot)M(dot) Cain" <darcy(at)druid(dot)net>
Cc: David Fetter <david(at)fetter(dot)org>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: New version of money type
Date: 2006-12-21 15:47:52
Message-ID: 19702.1166716072@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

"D'Arcy J.M. Cain" <darcy(at)druid(dot)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?
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.

One bug I see in it is that you'd better make the alignment 'd' if the
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.

(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.)

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2006-12-21 15:50:59 Re: column ordering, was Re: [PATCHES] Enums patch v2
Previous Message Martijn van Oosterhout 2006-12-21 15:37:03 Re: column ordering, was Re: [PATCHES] Enums patch v2