Thread: Re: [BUGS] wrong behavior using to_char()

Euler Taveira de Oliveira wrote:
> Jorge Godoy wrote:
>
> > > In the pt_BR locale, the thousand separator is "". So it should return
> >
> > The thousands separator in pt_BR is ".".
> >
> Oh, good catch. There is so much hack in my glibc. :-)

I researched this thread:

    http://archives.postgresql.org/pgsql-bugs/2006-09/msg00074.php

Ultimately, the result was that glibc was wrong in its locale settings,
and there was a suggestion to use defaults only when using the C locale.
However, I am worried there are too many locales in the field that only
define some of the locale setting, so doing defaults only for the C
locale might not work.

The minimal patch I wrote (attached), suppresses the default for the
thousands separator only if is is the same as the decimal separator.  I
think this is the only area where the default could potentially match
the locale setting for another field.

--
  Bruce Momjian  <bruce@momjian.us>          http://momjian.us
  EnterpriseDB                               http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/backend/utils/adt/formatting.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/formatting.c,v
retrieving revision 1.122
diff -c -c -r1.122 formatting.c
*** src/backend/utils/adt/formatting.c    9 Feb 2007 04:17:58 -0000    1.122
--- src/backend/utils/adt/formatting.c    9 Feb 2007 04:19:18 -0000
***************
*** 3835,3848 ****
              Np->L_positive_sign = "+";

          /*
-          * Number thousands separator
-          */
-         if (lconv->thousands_sep && *lconv->thousands_sep)
-             Np->L_thousands_sep = lconv->thousands_sep;
-         else
-             Np->L_thousands_sep = ",";
-
-         /*
           * Number decimal point
           */
          if (lconv->decimal_point && *lconv->decimal_point)
--- 3835,3840 ----
***************
*** 3850,3855 ****
--- 3842,3867 ----
          else
              Np->decimal = ".";

+         /* If they didn't ask for locale-specific decimal */
+         if (!IS_LDECIMAL(Np->Num))
+             Np->decimal = ".";
+
+         /*
+          * Number thousands separator
+          */
+         if (lconv->thousands_sep && *lconv->thousands_sep)
+             Np->L_thousands_sep = lconv->thousands_sep;
+         /*
+          *    Some locales (e.g. broken glibc pt_BR), have a comma for
+          *    decimal, but "" for thousands_sep, so we certainly
+          *    don't want to make the thousands_sep comma too, so
+          *    we just leave it blank.
+          */
+         else if (*Np->decimal != ',')
+             Np->L_thousands_sep = ",";
+         else
+             Np->L_thousands_sep = "";
+
          /*
           * Currency symbol
           */
***************
*** 3857,3865 ****
              Np->L_currency_symbol = lconv->currency_symbol;
          else
              Np->L_currency_symbol = " ";
-
-         if (!IS_LDECIMAL(Np->Num))
-             Np->decimal = ".";
      }
      else
      {
--- 3869,3874 ----

Bruce Momjian <bruce@momjian.us> writes:
> Ultimately, the result was that glibc was wrong in its locale settings,
> and there was a suggestion to use defaults only when using the C locale.
> However, I am worried there are too many locales in the field that only
> define some of the locale setting, so doing defaults only for the C
> locale might not work.

> The minimal patch I wrote (attached), suppresses the default for the
> thousands separator only if is is the same as the decimal separator.  I
> think this is the only area where the default could potentially match
> the locale setting for another field.

Should we really go introducing strange misbehaviors into our code to
work around an admitted glibc bug?

            regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Ultimately, the result was that glibc was wrong in its locale settings,
> > and there was a suggestion to use defaults only when using the C locale.
> > However, I am worried there are too many locales in the field that only
> > define some of the locale setting, so doing defaults only for the C
> > locale might not work.
>
> > The minimal patch I wrote (attached), suppresses the default for the
> > thousands separator only if is is the same as the decimal separator.  I
> > think this is the only area where the default could potentially match
> > the locale setting for another field.
>
> Should we really go introducing strange misbehaviors into our code to
> work around an admitted glibc bug?

Seems there is no interest in handling this specific case, so I withdraw
the patch, but I have added a comment with a date in case we need to
revisit it.

--
  Bruce Momjian  <bruce@momjian.us>          http://momjian.us
  EnterpriseDB                               http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +