Fix psql's code for locale-aware formatting of numeric output.

This code did the wrong thing entirely for numbers with an exponent
but no decimal point (e.g., '1e6'), as reported by Jeff Janes in
bug #13636.  More generally, it made lots of unverified assumptions
about what the input string could possibly look like.  Rearrange so
that it only fools with leading digits that it's directly verified
are there, and an immediately adjacent decimal point.  While at it,
get rid of some useless inefficiencies, like converting the grouping
count string to integer over and over (and over).

This has been broken for a long time, so back-patch to all supported
branches.
This commit is contained in:
Tom Lane 2015-09-24 23:01:04 -04:00
parent 39df0f150c
commit 4778a0bdaa
1 changed files with 49 additions and 58 deletions

View File

@ -39,8 +39,9 @@
*/ */
volatile bool cancel_pressed = false; volatile bool cancel_pressed = false;
/* info for locale-aware numeric formatting; set up by setDecimalLocale() */
static char *decimal_point; static char *decimal_point;
static char *grouping; static int groupdigits;
static char *thousands_sep; static char *thousands_sep;
static char default_footer[100]; static char default_footer[100];
@ -196,44 +197,35 @@ static void IsPagerNeeded(const printTableContent *cont, const int extra_lines,
static void print_aligned_vertical(const printTableContent *cont, FILE *fout); static void print_aligned_vertical(const printTableContent *cont, FILE *fout);
/* Count number of digits in integral part of number */
static int static int
integer_digits(const char *my_str) integer_digits(const char *my_str)
{ {
int frac_len; /* ignoring any sign ... */
if (my_str[0] == '-' || my_str[0] == '+')
if (my_str[0] == '-')
my_str++; my_str++;
/* ... count initial integral digits */
frac_len = strchr(my_str, '.') ? strlen(strchr(my_str, '.')) : 0; return strspn(my_str, "0123456789");
return strlen(my_str) - frac_len;
} }
/* Return additional length required for locale-aware numeric output */ /* Compute additional length required for locale-aware numeric output */
static int static int
additional_numeric_locale_len(const char *my_str) additional_numeric_locale_len(const char *my_str)
{ {
int int_len = integer_digits(my_str), int int_len = integer_digits(my_str),
len = 0; len = 0;
int groupdigits = atoi(grouping);
if (int_len > 0) /* Account for added thousands_sep instances */
/* Don't count a leading separator */ if (int_len > groupdigits)
len = (int_len / groupdigits - (int_len % groupdigits == 0)) * len += ((int_len - 1) / groupdigits) * strlen(thousands_sep);
strlen(thousands_sep);
/* Account for possible additional length of decimal_point */
if (strchr(my_str, '.') != NULL) if (strchr(my_str, '.') != NULL)
len += strlen(decimal_point) - strlen("."); len += strlen(decimal_point) - 1;
return len; return len;
} }
static int
strlen_with_numeric_locale(const char *my_str)
{
return strlen(my_str) + additional_numeric_locale_len(my_str);
}
/* /*
* Returns the appropriately formatted string in a new allocated block, * Returns the appropriately formatted string in a new allocated block,
* caller must free * caller must free
@ -241,53 +233,51 @@ strlen_with_numeric_locale(const char *my_str)
static char * static char *
format_numeric_locale(const char *my_str) format_numeric_locale(const char *my_str)
{ {
int new_len = strlen(my_str) + additional_numeric_locale_len(my_str);
char *new_str = pg_malloc(new_len + 1);
int int_len = integer_digits(my_str);
int i, int i,
j,
int_len = integer_digits(my_str),
leading_digits; leading_digits;
int groupdigits = atoi(grouping); int new_str_pos = 0;
int new_str_start = 0;
char *new_str = pg_malloc(strlen_with_numeric_locale(my_str) + 1);
leading_digits = (int_len % groupdigits != 0) ? /* number of digits in first thousands group */
int_len % groupdigits : groupdigits; leading_digits = int_len % groupdigits;
if (leading_digits == 0)
leading_digits = groupdigits;
if (my_str[0] == '-') /* skip over sign, affects grouping /* process sign */
* calculations */ if (my_str[0] == '-' || my_str[0] == '+')
{ {
new_str[0] = my_str[0]; new_str[new_str_pos++] = my_str[0];
my_str++; my_str++;
new_str_start = 1;
} }
for (i = 0, j = new_str_start;; i++, j++) /* process integer part of number */
for (i = 0; i < int_len; i++)
{ {
/* Hit decimal point? */ /* Time to insert separator? */
if (i > 0 && --leading_digits == 0)
{
strcpy(&new_str[new_str_pos], thousands_sep);
new_str_pos += strlen(thousands_sep);
leading_digits = groupdigits;
}
new_str[new_str_pos++] = my_str[i];
}
/* handle decimal point if any */
if (my_str[i] == '.') if (my_str[i] == '.')
{ {
strcpy(&new_str[j], decimal_point); strcpy(&new_str[new_str_pos], decimal_point);
j += strlen(decimal_point); new_str_pos += strlen(decimal_point);
/* add fractional part */ i++;
strcpy(&new_str[j], &my_str[i] + 1);
break;
} }
/* End of string? */ /* copy the rest (fractional digits and/or exponent, and \0 terminator) */
if (my_str[i] == '\0') strcpy(&new_str[new_str_pos], &my_str[i]);
{
new_str[j] = '\0';
break;
}
/* Add separator? */ /* assert we didn't underestimate new_len (an overestimate is OK) */
if (i != 0 && (i - leading_digits) % groupdigits == 0) Assert(strlen(new_str) <= new_len);
{
strcpy(&new_str[j], thousands_sep);
j += strlen(thousands_sep);
}
new_str[j] = my_str[i];
}
return new_str; return new_str;
} }
@ -3241,10 +3231,11 @@ setDecimalLocale(void)
decimal_point = pg_strdup(extlconv->decimal_point); decimal_point = pg_strdup(extlconv->decimal_point);
else else
decimal_point = "."; /* SQL output standard */ decimal_point = "."; /* SQL output standard */
if (*extlconv->grouping && atoi(extlconv->grouping) > 0) if (*extlconv->grouping && atoi(extlconv->grouping) > 0)
grouping = pg_strdup(extlconv->grouping); groupdigits = atoi(extlconv->grouping);
else else
grouping = "3"; /* most common */ groupdigits = 3; /* most common */
/* similar code exists in formatting.c */ /* similar code exists in formatting.c */
if (*extlconv->thousands_sep) if (*extlconv->thousands_sep)