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@hagander.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@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-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@postgresql.org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgsql-hackers
>>