Fix to_number() to correctly ignore thousands separator when it's '.'.

The existing code in NUM_numpart_from_char has hard-wired logic to treat
'.' as decimal point, even when we're using a locale-aware format string
and the locale says that '.' is the thousands separator.  This results in
clearly wrong answers in FM mode (where we must be able to identify the
decimal point location), as per bug report from Patryk Kordylewski.

Since the initialization code in NUM_prepare_locale already sets up
Np->decimal as either the locale decimal-point string or "." depending
on which decimal-point format code was used, there's really no need to
have any extra logic at all in NUM_numpart_from_char: we only need to
test for a match to Np->decimal.

(Note: AFAICS there's nothing in here that explicitly checks for thousands
separators --- rather, any unmatched character is silently skipped over.
That's pretty bogus IMO but it's not the issue being complained of.)

This is a longstanding bug, but it's possible that some existing apps
are depending on '.' being recognized as decimal point even when using
a D format code.  Hence, no back-patch.  We should probably list this
as a potential incompatibility in the 9.3 release notes.

diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index db5dfca51d477d3e9b33b8d2c264495b3b2ec433..81e3329ef60ce4f835fedba50208b8d0f4b19d63 100644 (file)
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -4131,7 +4131,7 @@ NUM_numpart_from_char(NUMProc *Np, int id, int plen)
 #endif
 
        /*
-        * read digit
+        * read digit or decimal point
         */
        if (isdigit((unsigned char) *Np->inout_p))
        {
@@ -4151,40 +4151,28 @@ NUM_numpart_from_char(NUMProc *Np, int id, int plen)
 #ifdef DEBUG_TO_FROM_CHAR
                elog(DEBUG_elog_output, "Read digit (%c)", *Np->inout_p);
 #endif
-
-               /*
-                * read decimal point
-                */
        }
        else if (IS_DECIMAL(Np->Num) && Np->read_dec == FALSE)
        {
+               /*
+                * We need not test IS_LDECIMAL(Np->Num) explicitly here, because
+                * Np->decimal is always just "." if we don't have a D format token.
+                * So we just unconditionally match to Np->decimal.
+                */
+               int                     x = strlen(Np->decimal);
+
 #ifdef DEBUG_TO_FROM_CHAR
-               elog(DEBUG_elog_output, "Try read decimal point (%c)", *Np->inout_p);
+               elog(DEBUG_elog_output, "Try read decimal point (%c)",
+                        *Np->inout_p);
 #endif
-               if (*Np->inout_p == '.')
+               if (x && AMOUNT_TEST(x) && strncmp(Np->inout_p, Np->decimal, x) == 0)
                {
+                       Np->inout_p += x - 1;
                        *Np->number_p = '.';
                        Np->number_p++;
                        Np->read_dec = TRUE;
                        isread = TRUE;
                }
-               else
-               {
-                       int                     x = strlen(Np->decimal);
-
-#ifdef DEBUG_TO_FROM_CHAR
-                       elog(DEBUG_elog_output, "Try read locale point (%c)",
-                                *Np->inout_p);
-#endif
-                       if (x && AMOUNT_TEST(x) && strncmp(Np->inout_p, Np->decimal, x) == 0)
-                       {
-                               Np->inout_p += x - 1;
-                               *Np->number_p = '.';
-                               Np->number_p++;
-                               Np->read_dec = TRUE;
-                               isread = TRUE;
-                       }
-               }
        }
 
        if (OVERLOAD_TEST)