Re: [HACKERS] wrong behavior using to_char() again - Mailing list pgsql-patches

From Bruce Momjian
Subject Re: [HACKERS] wrong behavior using to_char() again
Date
Msg-id 200711212236.lALMaQg13337@momjian.us
Whole thread Raw
Responses Re: [HACKERS] wrong behavior using to_char() again
List pgsql-patches
Euler Taveira de Oliveira wrote:
> Hi,
>
> Looking again at bug report [1], I agree that's a glibc bug. Numbers in
> pt_BR has its format 1.234.567,89; sometimes the format 1234567,89 is
> acceptable too, ie, the thousand separator is optional. I guess that
> some locales use the 'optional' thousand separator too (yep, they are
> all broken too).
>
> euler@harman:/a/pgsql$ ./a.out pt_BR
> decimal_point: ,
> thousands_sep:
> euler@harman:/a/pgsql$ ./a.out fr_FR
> decimal_point: ,
> thousands_sep:
> euler@harman:/a/pgsql$ ./a.out es_ES
> decimal_point: ,
> thousands_sep:
> euler@harman:/a/pgsql$ ./a.out de_DE
> decimal_point: ,
> thousands_sep: .
> euler@harman:/a/pgsql$ ./a.out C
> decimal_point: .
> thousands_sep:
>
> The actual behavior is set: (i) "," if the thousand separator is "" (ii)
> "." if the decimal point is "". It is not what glibc says (even in the C
> locale). I expect that PostgreSQL agrees with glibc (even it's the wrong
> behavior). Given this assumption, i propose the attached patch (it needs
> to adjust the regression tests).

OK, I researched this and realized it should have been obvious to me
when I added this code in 2006 that making the thousands separator
always "," for a locale of "" was going to cause a problem.

What I have done is to duplicate the logic we already had in
psql/print.c where the default thousands separator is either "," or "."
to make it different from the decimal separator.

I stated in the thread I would look at it for 8.3 but forgot:

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

I don't think there is any change needed for the C locale.  That part
seems fine, as Alvaro already pointed out.

Please test, patch attached and applied to CVS.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://postgres.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.133
diff -c -c -r1.133 formatting.c
*** src/backend/utils/adt/formatting.c    21 Nov 2007 21:49:22 -0000    1.133
--- src/backend/utils/adt/formatting.c    21 Nov 2007 22:24:52 -0000
***************
*** 3915,3921 ****
           */
          if (lconv->decimal_point && *lconv->decimal_point)
              Np->decimal = lconv->decimal_point;
-
          else
              Np->decimal = ".";

--- 3915,3920 ----
***************
*** 3926,3938 ****
           * Number thousands separator
           *
           * Some locales (e.g. broken glibc pt_BR), have a comma for decimal,
!          * but "" for thousands_sep, so we make the thousands_sep comma
!          * too.  2007-02-12
           */
          if (lconv->thousands_sep && *lconv->thousands_sep)
              Np->L_thousands_sep = lconv->thousands_sep;
!         else
              Np->L_thousands_sep = ",";

          /*
           * Currency symbol
--- 3925,3938 ----
           * Number thousands separator
           *
           * Some locales (e.g. broken glibc pt_BR), have a comma for decimal,
!          * but "" for thousands_sep, so we set the thousands_sep too. 2007-02-12
           */
          if (lconv->thousands_sep && *lconv->thousands_sep)
              Np->L_thousands_sep = lconv->thousands_sep;
!         else if (strcmp(Np->decimal, ",") != 0)
              Np->L_thousands_sep = ",";
+         else
+             Np->L_thousands_sep = ".";

          /*
           * Currency symbol
Index: src/bin/psql/print.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/print.c,v
retrieving revision 1.91
diff -c -c -r1.91 print.c
*** src/bin/psql/print.c    5 Jan 2007 22:19:49 -0000    1.91
--- src/bin/psql/print.c    21 Nov 2007 22:24:53 -0000
***************
*** 2039,2045 ****
          grouping = "3";            /* most common */
      if (*extlconv->thousands_sep)
          thousands_sep = strdup(extlconv->thousands_sep);
!     else if (*decimal_point != ',')
          thousands_sep = ",";
      else
          thousands_sep = ".";
--- 2039,2045 ----
          grouping = "3";            /* most common */
      if (*extlconv->thousands_sep)
          thousands_sep = strdup(extlconv->thousands_sep);
!     else if (strcmp(decimal_point, ",") != 0)
          thousands_sep = ",";
      else
          thousands_sep = ".";

pgsql-patches by date:

Previous
From: Matteo Beccati
Date:
Subject: Re: Better default_statistics_target
Next
From: Bruce Momjian
Date:
Subject: Re: Fix pg_dump dependency on postgres.h