"D'Arcy J.M. Cain" <email@example.com> 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 ---------------------------(end of broadcast)--------------------------- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly