From 35d50b527a9f99e22a317269ceb00491397d0e00 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 11 May 2013 16:35:03 -0400 Subject: [PATCH] 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. --- src/backend/utils/adt/formatting.c | 36 ++++++++++-------------------- 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c index db5dfca51d..81e3329ef6 100644 --- 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)