Re: Fixes for MONEY type using locale

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: "D'Arcy J(dot)M(dot) Cain" <darcy(at)druid(dot)net>
Cc: PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: Fixes for MONEY type using locale
Date: 2007-11-24 16:27:38
Message-ID: 200711241627.lAOGRc504368@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

D'Arcy J.M. Cain wrote:
> On Fri, 23 Nov 2007 15:59:29 -0500 (EST)
> Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> > I also removed a ssymbol test because ssymbol will never be \0 based on
> > the code. Also, what does this do:
> >
> > if (buf[LAST_DIGIT] == ',')
> > buf[LAST_DIGIT] = buf[LAST_PAREN];
>
> Good question. This has been in the code since day one. I had to go
> back to the book it came from (Software Solutions in C - ISBN
> 0-12-632360-7) to see what I wrote back in 1994. Here is what the text
> says about that.
>
> "The next part may look a little strange, but the previous code is a
> little simpler if we just put a comma at the end of the string when
> there are no decimal points and strip it off here. Otherwise we would
> have to adjust the comma_position variable and test in the main loop
> for the special case."
>
> The if block has a second statement though which is effectively this;
>
> buf[LAST_PAREN] = ' ';
>
> This is followed by;
>
> "The Two assignments deal with the case where there is a trailing
> parenthesis without having to do another test."
>
> So that's what I know now. I'm not sure what the edge case is. For
> one thing it would only be triggered in locales with no places after
> the decimal point. I'll try to work up some test later.

Your summary is helpful. I ran some tests with points == 0 and number
of digits % mon_group == 0 and did see the trailing ssymbol before that
code, and it removed after. What they are doing is setting
buf[LAST_PAREN] to the NUL byte above the ssymbol code, and then at this
point they just trim off the ssymbol. (Perhaps the original code put the
parentheses in earlier and had to do this trick to shift in the
parentheses, but we do it after this block.)

What I did was to hard-code the \0 in there and add a comment about what
it is doing --- patch attached and applied.

I am confused about two other items with MONEY. First, why can't
anything but a string be cast to this type?

test=> select 871234872319489323::money;
ERROR: cannot cast type bigint to money
LINE 1: select 871234872319489323::money;
^
test=> select 871234872::money;
ERROR: cannot cast type integer to money
LINE 1: select 871234872::money;
^
test=> select 87123487231.3::money;
ERROR: cannot cast type numeric to money
LINE 1: select 87123487231.3::money;
^

Without such castings, how is this data type used? I did a \dC and saw
no entries for MONEY.

And second, why are there no regression tests for MONEY. I see it used
only once in the rules test.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Attachment Content-Type Size
/rtmp/diff text/x-diff 1.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Guillaume Smet 2007-11-24 16:41:37 Re: 8.3devel slower than 8.2 under read-only load
Previous Message Gregory Stark 2007-11-24 16:16:27 Re: 8.3devel slower than 8.2 under read-only load

Browse pgsql-patches by date

  From Date Subject
Next Message D'Arcy J.M. Cain 2007-11-24 17:14:15 Re: Fixes for MONEY type using locale
Previous Message Bruce Momjian 2007-11-24 15:28:54 Re: Fixes for MONEY type using locale