"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?
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