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

Does it work when you're not using C-locale? This looks like the
codepath Tom pointed out was wrong.

//Magnus

Hiroshi Saito wrote:
> 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
>>