Re: [PATCHES] Solve a problem of LC_TIME of windows. - Mailing list pgsql-hackers

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
>