Re: [GENERAL] trouble with to_char('L') - Mailing list pgsql-hackers

From Bruce Momjian
Subject Re: [GENERAL] trouble with to_char('L')
Date
Msg-id 201004242303.o3ON3Sk05147@momjian.us
Whole thread Raw
In response to Re: [GENERAL] trouble with to_char('L')  (Takahiro Itagaki <itagaki.takahiro@oss.ntt.co.jp>)
Responses Re: [GENERAL] trouble with to_char('L')  (Hiroshi Inoue <inoue@tpf.co.jp>)
List pgsql-hackers
Takahiro Itagaki wrote:
>
> Takahiro Itagaki <itagaki.takahiro@oss.ntt.co.jp> wrote:
>
> > Revised patch attached. Please test it.
>
> I applied this version of the patch.
> Please check wheter the bug is fixed and any buildfarm failures.

Great.  I have merged in my C comments into the code with the attached
patch so we remember why the code is setup as it is.

One thing I am confused about is that, for Win32, our numeric/monetary
handling sets lc_ctype to match numeric/monetary, while our time code in
the same file uses that method _and_ uses wcsftime() to return the value
in wide characters.  So, why do we do both for time?  Is there any value
to that?

Seems we should do the same for both numeric/monetary and time.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com
Index: src/backend/utils/adt/pg_locale.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/pg_locale.c,v
retrieving revision 1.54
diff -c -c -r1.54 pg_locale.c
*** src/backend/utils/adt/pg_locale.c    22 Apr 2010 01:55:52 -0000    1.54
--- src/backend/utils/adt/pg_locale.c    24 Apr 2010 22:43:53 -0000
***************
*** 41,46 ****
--- 41,50 ----
   * DOES NOT WORK RELIABLY: on some platforms the second setlocale() call
   * will change the memory save is pointing at.    To do this sort of thing
   * safely, you *must* pstrdup what setlocale returns the first time.
+  *
+  * FYI, The Open Group locale standard is defined here:
+  *
+  *  http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap07.html
   *----------
   */

***************
*** 424,430 ****
      char       *grouping;
      char       *thousands_sep;
      int            encoding;
-
  #ifdef WIN32
      char       *save_lc_ctype;
  #endif
--- 428,433 ----
***************
*** 435,459 ****

      free_struct_lconv(&CurrentLocaleConv);

!     /* Set user's values of monetary and numeric locales */
      save_lc_monetary = setlocale(LC_MONETARY, NULL);
      if (save_lc_monetary)
          save_lc_monetary = pstrdup(save_lc_monetary);
      save_lc_numeric = setlocale(LC_NUMERIC, NULL);
      if (save_lc_numeric)
          save_lc_numeric = pstrdup(save_lc_numeric);

  #ifdef WIN32
!     /* set user's value of ctype locale */
      save_lc_ctype = setlocale(LC_CTYPE, NULL);
      if (save_lc_ctype)
          save_lc_ctype = pstrdup(save_lc_ctype);
- #endif

!     /* Get formatting information for numeric */
! #ifdef WIN32
      setlocale(LC_CTYPE, locale_numeric);
  #endif
      setlocale(LC_NUMERIC, locale_numeric);
      extlconv = localeconv();
      encoding = pg_get_encoding_from_locale(locale_numeric);
--- 438,485 ----

      free_struct_lconv(&CurrentLocaleConv);

!     /* Save user's values of monetary and numeric locales */
      save_lc_monetary = setlocale(LC_MONETARY, NULL);
      if (save_lc_monetary)
          save_lc_monetary = pstrdup(save_lc_monetary);
+
      save_lc_numeric = setlocale(LC_NUMERIC, NULL);
      if (save_lc_numeric)
          save_lc_numeric = pstrdup(save_lc_numeric);

  #ifdef WIN32
!    /*
!     *  Ideally, monetary and numeric local symbols could be returned in
!     *  any server encoding.  Unfortunately, the WIN32 API does not allow
!     *  setlocale() to return values in a codepage/CTYPE that uses more
!     *  than two bytes per character, like UTF-8:
!     *
!     *      http://msdn.microsoft.com/en-us/library/x99tb11d.aspx
!     *
!     *  Evidently, LC_CTYPE allows us to control the encoding used
!     *  for strings returned by localeconv().  The Open Group
!     *  standard, mentioned at the top of this C file, doesn't
!     *  explicitly state this.
!     *
!     *  Therefore, we set LC_CTYPE to match LC_NUMERIC or LC_MONETARY
!     *  (which cannot be UTF8), call localeconv(), and then convert from
!     *  the numeric/monitary LC_CTYPE to the server encoding.  One
!     *  example use of this is for the Euro symbol.
!     *
!     *  Perhaps someday we will use GetLocaleInfoW() which returns values
!     *  in UTF16 and convert from that.
!     */
!
!     /* save user's value of ctype locale */
      save_lc_ctype = setlocale(LC_CTYPE, NULL);
      if (save_lc_ctype)
          save_lc_ctype = pstrdup(save_lc_ctype);

!     /* use numeric to set the ctype */
      setlocale(LC_CTYPE, locale_numeric);
  #endif
+
+     /* Get formatting information for numeric */
      setlocale(LC_NUMERIC, locale_numeric);
      extlconv = localeconv();
      encoding = pg_get_encoding_from_locale(locale_numeric);
***************
*** 462,471 ****
      thousands_sep = db_encoding_strdup(encoding, extlconv->thousands_sep);
      grouping = strdup(extlconv->grouping);

-     /* Get formatting information for monetary */
  #ifdef WIN32
      setlocale(LC_CTYPE, locale_monetary);
  #endif
      setlocale(LC_MONETARY, locale_monetary);
      extlconv = localeconv();
      encoding = pg_get_encoding_from_locale(locale_monetary);
--- 488,499 ----
      thousands_sep = db_encoding_strdup(encoding, extlconv->thousands_sep);
      grouping = strdup(extlconv->grouping);

  #ifdef WIN32
+     /* use monetary to set the ctype */
      setlocale(LC_CTYPE, locale_monetary);
  #endif
+
+     /* Get formatting information for monetary */
      setlocale(LC_MONETARY, locale_monetary);
      extlconv = localeconv();
      encoding = pg_get_encoding_from_locale(locale_monetary);
***************
*** 500,506 ****
      }

  #ifdef WIN32
!     /* try to restore internal ctype settings */
      if (save_lc_ctype)
      {
          setlocale(LC_CTYPE, save_lc_ctype);
--- 528,534 ----
      }

  #ifdef WIN32
!     /* Try to restore internal ctype settings */
      if (save_lc_ctype)
      {
          setlocale(LC_CTYPE, save_lc_ctype);
***************
*** 514,526 ****

  #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.
   *
   * Note that this only affects the calls to strftime() in this file, which are
   * used to get the locale-aware strings. Other parts of the backend use
--- 542,556 ----

  #ifdef WIN32
  /*
!  * On WIN32, strftime() returns the encoding in CP_ACP (the default
!  * operating system codpage for that computer), 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.
   *
!  * So, instead of using strftime(), use wcsftime() to return the value in
!  * wide characters (internally UTF16) and then convert it to the appropriate
!  * database encoding.
   *
   * Note that this only affects the calls to strftime() in this file, which are
   * used to get the locale-aware strings. Other parts of the backend use
***************
*** 537,543 ****

      len = wcsftime(wbuf, MAX_L10N_DATA, format, tm);
      if (len == 0)
-
          /*
           * strftime call failed - return 0 with the contents of dst
           * unspecified
--- 567,572 ----
***************
*** 564,570 ****
--- 593,601 ----
      return len;
  }

+ /* redefine strftime() */
  #define strftime(a,b,c,d) strftime_win32(a,b,L##c,d)
+
  #endif   /* WIN32 */


***************
*** 580,586 ****
      char        buf[MAX_L10N_DATA];
      char       *ptr;
      int            i;
-
  #ifdef WIN32
      char       *save_lc_ctype;
  #endif
--- 611,616 ----
***************
*** 591,610 ****

      elog(DEBUG3, "cache_locale_time() executed; locale: \"%s\"", locale_time);

  #ifdef WIN32
!     /* set user's value of ctype locale */
      save_lc_ctype = setlocale(LC_CTYPE, NULL);
      if (save_lc_ctype)
          save_lc_ctype = pstrdup(save_lc_ctype);

      setlocale(LC_CTYPE, locale_time);
  #endif

-     /* set user's value of time locale */
-     save_lc_time = setlocale(LC_TIME, NULL);
-     if (save_lc_time)
-         save_lc_time = pstrdup(save_lc_time);
-
      setlocale(LC_TIME, locale_time);

      timenow = time(NULL);
--- 621,642 ----

      elog(DEBUG3, "cache_locale_time() executed; locale: \"%s\"", locale_time);

+     /* save user's value of time locale */
+     save_lc_time = setlocale(LC_TIME, NULL);
+     if (save_lc_time)
+         save_lc_time = pstrdup(save_lc_time);
+
  #ifdef WIN32
!     /* See the WIN32 comment near the top of PGLC_localeconv() */
!     /* save user's value of ctype locale */
      save_lc_ctype = setlocale(LC_CTYPE, NULL);
      if (save_lc_ctype)
          save_lc_ctype = pstrdup(save_lc_ctype);

+     /* use lc_time to set the ctype */
      setlocale(LC_CTYPE, locale_time);
  #endif

      setlocale(LC_TIME, locale_time);

      timenow = time(NULL);

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: global temporary tables
Next
From: Bruce Momjian
Date:
Subject: Re: [RFC] nodeToString format and exporting the SQL parser