Re: [PATCHES] Solve a problem of LC_TIME of windows.

From: "Hiroshi Saito" <z-saito(at)guitar(dot)ocn(dot)ne(dot)jp>
To: "Magnus Hagander" <magnus(at)hagander(dot)net>
Cc: "ITAGAKI Takahiro" <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, "Jaime Casanova" <jcasanov(at)systemguards(dot)com(dot)ec>, "Alvaro Herrera" <alvherre(at)commandprompt(dot)com>, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "pgsql-hackers" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCHES] Solve a problem of LC_TIME of windows.
Date: 2008-11-24 14:48:22
Message-ID: E0B068FB60364D3E87C384757B4F9416@HIRO57887DE653
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

Hi Magnus-san.

Um, Though regrettable, it is not in a good state.....
http://winpg.jp/~saito/pg_work/LC_MESSAGE_CHECK/LC_TIME_PATCH/Mugnus-patch.png

----- Original Message -----
From: "Magnus Hagander" <magnus(at)hagander(dot)net>

> The way I read this patch we do:
> 1) Get the time in CP_ACP
> 2) Convert to UTF16
> 3) Convert to UTF8
> 4) If necessary, convert to the charset in the database
>
> We could be more efficient by at least folding 1 and 2 into a single
> step, which means getting it in UTF16 from the beginning. How about
> attached patch? It builds, but please verify that it fixes the problem.
>
> (I've updated the comment as well)
>
> Attached pg_locale_utf16.patch. I'm also attaching
> pg_locale_diffdiff.patch which contains the changes I've made against
> your patch only.
>
> //Magnus
>
>
> Hiroshi Saito wrote:
>> Hi.
>>
>> All suggestion is appropriate and has been checked.
>>
>> CVS-HEAD was examined by MinGW. $ make check NO_LOCALE=true
>> ...
>> =======================
>> All 118 tests passed. =======================
>>
>> Then, It continues and a review is desired. Thanks!
>>
>> Regatrds,
>> Hiroshi Saito
>>
>> ----- Original Message ----- From: "Hiroshi Saito"
>> <z-saito(at)guitar(dot)ocn(dot)ne(dot)jp>
>>
>>
>>> Hi ITAGAKI-san.
>>>
>>> Sorry, very late reaction..
>>> I lost time resources in an individual my machine trouble and
>>> busyness.:-(
>>> Now, I appreciate your exact work. ! Then, I desire the best patch for
>>> PostgreSQL. Probably, I think that it is finally helpful in not a
>>> problem of only Japan but many countries. Tom-san, and Alvaro-san,
>>> Magnus-san understands the essence of this problem. Therefore, the
>>> suggestion is expected for me.
>>> Anyway, thank you very much.!!
>>>
>>> Regards,
>>> Hiroshi Saito
>>>
>>> ----- Original Message ----- From: "ITAGAKI Takahiro"
>>> <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>
>>>
>>>
>>>>
>>>> "Jaime Casanova" <jcasanov(at)systemguards(dot)com(dot)ec> wrote:
>>>>
>>>>> i'm confused, original patch has this signature:
>>>>> + conv_strftime(char *src, size_t len, const char *format, const
>>>>> struct tm *tm)
>>>>> your's has:
>>>>> +strftime_win32(char *dst, size_t dstlen, const char *format, const
>>>>
>>>>> you change all src for dst, just a variable name decision but a
>>>>> radical one... why was that (i honestly doesn't understand this patch
>>>>> very well ;)?
>>>>
>>>> That's because the first argument is not an input buffer,
>>>> but an output buffer. MSDN also calls it 'strDest'.
>>>> http://msdn.microsoft.com/en-us/library/fe06s4ak(VS.71).aspx
>>>> Linux manpage calls it 's', but I think it means String, not Src.
>>>> http://man.cx/strftime
>>>>
>>>> If you can review the patch, please use the attached one instead.
>>>> I modified it in response to the discussion of
>>>> pg_do_encoding_conversion.
>>>>
>>>>
>>>> BTW, I cannot understand the comment in the function head,
>>>>
>>>> + * result is obtained by locale setup of LC_TIME in the environment
>>>> + * of windows at present CP_ACP. Therefore, conversion is needed
>>>> + * for SERVER_ENCODING. SJIS which is not especially made to server
>>>> + * encoding in Japan returns.
>>>> but it probably says:
>>>>
>>>> ----
>>>> strftime in Windows returns in CP_ACP encoding, but it could be
>>>> different from SERVER_ENCODING. Especially, Windows Japanese edition
>>>> requires conversions because it uses SJIS as CP_ACP, but we don't
>>>> support SJIS as a server encoding.
>>>> ----
>>>>
>>>> I hope you would review my English not only C ;-)
>>>>
>>>> Regards,
>>>> ---
>>>> ITAGAKI Takahiro
>>>> NTT Open Source Software Center
>>>>
>>>>
>>>
>>> --
>>> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
>>> To make changes to your subscription:
>>> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

--------------------------------------------------------------------------------

> *** a/src/backend/utils/adt/pg_locale.c
> --- b/src/backend/utils/adt/pg_locale.c
> ***************
> *** 54,59 ****
> --- 54,60 ----
> #include "utils/memutils.h"
> #include "utils/pg_locale.h"
>
> + #include "mb/pg_wchar.h"
>
> #define MAX_L10N_DATA 80
>
> ***************
> *** 452,457 **** PGLC_localeconv(void)
> --- 453,507 ----
> return &CurrentLocaleConv;
> }
>
> + #ifdef WIN32
> + /*
> + * On win32, strftime() returns the encoding in CP_ACP, which is likely
> + * different from SERVER_ENCODING. This is especially important in Japanese
> + * versions of Windows which will use SJIS encoding, which we don't support
> + * as a server encoding.
> + *
> + * Replace strftime() with a version that gets the string in UTF16 and then
> + * converts it to the appropriate encoding as necessary.
> + */
> + static size_t
> + strftime_win32(char *dst, size_t dstlen, const char *format, const struct tm *tm)
> + {
> + size_t len;
> + wchar_t wbuf[MAX_L10N_DATA];
> + int encoding;
> +
> + encoding = GetDatabaseEncoding();
> + if (encoding == PG_SQL_ASCII)
> + return len;
> +
> + len = wcsftime(wbuf, sizeof(wbuf), format, tm);
> + if (len == 0)
> + /* strftime called failed - return 0 with the contents of dst unspecified */
> + return 0;
> +
> + len = WideCharToMultiByte(CP_UTF8, 0, wbuf, len, dst, dstlen, NULL, NULL);
> + if (len == 0)
> + ereport(ERROR,
> + (errmsg("could not convert string to UTF-8:error %lu", GetLastError())));
> +
> + dst[len] = '\0';
> + if (encoding != PG_UTF8)
> + {
> + char *convstr = pg_do_encoding_conversion(dst, len, PG_UTF8, encoding);
> + if (dst != convstr)
> + {
> + StrNCpy(dst, convstr, dstlen);
> + len = strlen(dst);
> + }
> + }
> +
> + return len;
> + }
> +
> + #define strftime(a,b,c,d) strftime_win32(a,b,c,d)
> +
> + #endif /* WIN32 */
> +
>
> /*
> * Update the lc_time localization cache variables if needed.
>

--------------------------------------------------------------------------------

> diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
> index f78a80f..c37ddd5 100644
> --- a/src/backend/utils/adt/pg_locale.c
> +++ b/src/backend/utils/adt/pg_locale.c
> @@ -455,10 +455,13 @@ PGLC_localeconv(void)
>
> #ifdef WIN32
> /*
> - * result is obtained by locale setup of LC_TIME in the environment
> - * of windows at present CP_ACP. Therefore, conversion is needed
> - * for SERVER_ENCODING. SJIS which is not especially made to server
> - * encoding in Japan returns.
> + * On win32, strftime() returns the encoding in CP_ACP, which is likely
> + * different from SERVER_ENCODING. This is especially important in Japanese
> + * versions of Windows which will use SJIS encoding, which we don't support
> + * as a server encoding.
> + *
> + * Replace strftime() with a version that gets the string in UTF16 and then
> + * converts it to the appropriate encoding as necessary.
> */
> static size_t
> strftime_win32(char *dst, size_t dstlen, const char *format, const struct tm *tm)
> @@ -467,19 +470,19 @@ strftime_win32(char *dst, size_t dstlen, const char *format, const struct
> tm *tm
> wchar_t wbuf[MAX_L10N_DATA];
> int encoding;
>
> - len = strftime(dst, dstlen, format, tm);
> encoding = GetDatabaseEncoding();
> if (encoding == PG_SQL_ASCII)
> return len;
>
> - len = MultiByteToWideChar(CP_ACP, 0, dst, len, wbuf, MAX_L10N_DATA);
> + len = wcsftime(wbuf, sizeof(wbuf), format, tm);
> if (len == 0)
> - ereport(ERROR,
> - (errmsg("could not convert string to wide character:error %lu", GetLastError())));
> + /* strftime called failed - return 0 with the contents of dst unspecified */
> + return 0;
> +
> len = WideCharToMultiByte(CP_UTF8, 0, wbuf, len, dst, dstlen, NULL, NULL);
> if (len == 0)
> ereport(ERROR,
> - (errmsg("could not convert wide character to UTF-8:error %lu", GetLastError())));
> + (errmsg("could not convert string to UTF-8:error %lu", GetLastError())));
>
> dst[len] = '\0';
> if (encoding != PG_UTF8)
>

--------------------------------------------------------------------------------

>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2008-11-24 14:50:34 Re: [PATCHES] Solve a problem of LC_TIME of windows.
Previous Message Magnus Hagander 2008-11-24 14:48:16 Re: Autoconf, libpq and replacement function

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2008-11-24 14:50:34 Re: [PATCHES] Solve a problem of LC_TIME of windows.
Previous Message Magnus Hagander 2008-11-24 13:02:05 Re: [PATCHES] Solve a problem of LC_TIME of windows.