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.
This commit is contained in:
Tom Lane 2013-05-11 16:35:03 -04:00
parent 8cade04c10
commit 35d50b527a
1 changed files with 12 additions and 24 deletions

View File

@ -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)