Re: [GENERAL] trouble with to_char('L')

From: Hiroshi Inoue <inoue(at)tpf(dot)co(dot)jp>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-hackers(at)postgreSQL(dot)org
Subject: Re: [GENERAL] trouble with to_char('L')
Date: 2009-06-02 03:22:13
Message-ID: 4A249AE5.9060006@tpf.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-general pgsql-hackers

Tom Lane wrote:
> Hiroshi Inoue <inoue(at)tpf(dot)co(dot)jp> writes:
>> Tom Lane wrote:
>>> I think what this suggests is that there probably needs to be some
>>> encoding conversion logic near the places we examine localeconv()
>>> output.
>
>> Attached is a patch to the current CVS.
>> It uses a similar way like LC_TIME stuff does.
>
> I'm not really in a position to test/commit this, since I don't have a
> Windows machine. However, since no one else is stepping up to deal with
> it, here's a quick review:

Thanks for the review.
I've forgotten the patch because Japanese doesn't have trouble with
this issue (the currency symbol is ascii \). If this is really
expected to be fixed, I would update the patch according to your
suggestion.

> * This seems to be assuming that the user has set LC_MONETARY and
> LC_NUMERIC the same. What if they're different?

Strictky speaking they should be handled individually.

> * What if the selected locale corresponds to Unicode (ie UTF16)
> encoding?

As far as I tested set_locale(LC_MONETARY, xxx.65001) causes an error.

> * #define'ing strdup() to do something rather different from strdup
> seems pretty horrid from the standpoint of code readability and
> maintainability, especially with nary a comment explaining it.

Maybe using a function instead of strdup() which calls dbstr_win32()
in case of Windows would be better.
BTW grouping and money_grouping seem to be out of encoding conversion.
Are they guaranteed to be null terminated?

> * Code will dump core on malloc failure.

I can take care of it.

> * Since this code is surely not performance critical, I wouldn't bother
> with trying to optimize it; hence drop the special case for all-ASCII.

I can take care of it.
>
> * Surely we already have a symbol somewhere that can be used in
> place of this:
> #define MAX_BYTES_PER_CHARACTER 4

I can't find it.
max(pg_encoding_max_length(encoding), pg_encoding_max_length(PG_UTF8))
may be better.

regards,
Hiroshi Inoue

In response to

Responses

Browse pgsql-general by date

  From Date Subject
Next Message Douglas Alan 2009-06-02 07:09:55 Re: How can I manually alter the statistics for a column?
Previous Message Scott Bailey 2009-06-02 01:55:04 Re: ruby connect

Browse pgsql-hackers by date

  From Date Subject
Next Message Joe Conway 2009-06-02 03:25:06 Re: dblink patches for comment
Previous Message Fujii Masao 2009-06-02 02:39:36 Re: pg_standby -l might destory the archived file