From: Magnus Hagander <magnus@hagander.net>
To: Hiroshi Saito <z-saito@guitar.ocn.ne.jp>
Cc: ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
Cc: Jaime Casanova <jcasanov@systemguards.com.ec>
Cc: Alvaro Herrera <alvherre@commandprompt.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: [PATCHES] Solve a problem of LC_TIME of windows.
Date: Mon, 24 Nov 2008 14:02:05 +0100
Message-ID: <492AA5CD.8050001@hagander.net> (raw)
In-Reply-To: <446F4763B989489C9B626D21AB9F6C7F@HIRO57887DE653>
References: <20081104094301.7EE8.52131E4D@oss.ntt.co.jp>
<3073cc9b0811102212n278644a8o7169f7c5ea562081@mail.gmail.com>
<20081111152112.B605.52131E4D@oss.ntt.co.jp>
<A32F4AB7D95646B3A358002E1FB73F9C@HIRO57887DE653>
<446F4763B989489C9B626D21AB9F6C7F@HIRO57887DE653>
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@guitar.ocn.ne.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.takahiro@oss.ntt.co.jp>
>>
>>
>>>
>>> "Jaime Casanova" <jcasanov@systemguards.com.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@postgresql.org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgsql-hackersAttachments:download all (2 files, .tar.gz)pg_locale_utf16.patch (text/x-diff, 1.8K) download
*** 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.
pg_locale_diffdiff.patch (text/x-diff, 1.9K) downloaddiff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.cindex 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)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-patches@postgresql.org
Cc: magnus@hagander.net, z-saito@guitar.ocn.ne.jp, itagaki.takahiro@oss.ntt.co.jp, jcasanov@systemguards.com.ec, alvherre@commandprompt.com, tgl@sss.pgh.pa.us, pgsql-hackers@postgresql.org
Subject: Re: [PATCHES] Solve a problem of LC_TIME of windows.
In-Reply-To: <492AA5CD.8050001@hagander.net>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox