Skip to content

Commit 7e327ec

Browse files
tglsfdctglsfdc
committed
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.
1 parent b7dcb2d commit 7e327ec

File tree

1 file changed

+45
-57
lines changed

1 file changed

+45
-57
lines changed

‎src/bin/psql/print.c

Lines changed: 45 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,9 @@
3939
*/
4040
volatile bool cancel_pressed = false;
4141

42+
/* info for locale-aware numeric formatting; set up by setDecimalLocale() */
4243
static char *decimal_point;
43-
static char *grouping;
44+
static int groupdigits;
4445
static char *thousands_sep;
4546

4647
static char default_footer[100];
@@ -130,99 +131,85 @@ static void IsPagerNeeded(const printTableContent *cont, const int extra_lines,
130131
static void print_aligned_vertical(const printTableContent *cont, FILE *fout);
131132

132133

134+
/* Count number of digits in integral part of number */
133135
static int
134136
integer_digits(const char *my_str)
135137
{
136-
int frac_len;
137-
138-
if (my_str[0] == '-')
138+
/* ignoring any sign ... */
139+
if (my_str[0] == '-' || my_str[0] == '+')
139140
my_str++;
140-
141-
frac_len = strchr(my_str, '.') ? strlen(strchr(my_str, '.')) : 0;
142-
143-
return strlen(my_str) - frac_len;
141+
/* ... count initial integral digits */
142+
return strspn(my_str, "0123456789");
144143
}
145144

146-
/* Return additional length required for locale-aware numeric output */
145+
/* Compute additional length required for locale-aware numeric output */
147146
static int
148147
additional_numeric_locale_len(const char *my_str)
149148
{
150149
int int_len = integer_digits(my_str),
151150
len = 0;
152-
int groupdigits = atoi(grouping);
153151

154-
if (int_len > 0)
155-
/* Don't count a leading separator */
156-
len = (int_len / groupdigits - (int_len % groupdigits == 0)) *
157-
strlen(thousands_sep);
152+
/* Account for added thousands_sep instances */
153+
if (int_len > groupdigits)
154+
len += ((int_len - 1) / groupdigits) * strlen(thousands_sep);
158155

156+
/* Account for possible additional length of decimal_point */
159157
if (strchr(my_str, '.') != NULL)
160-
len += strlen(decimal_point) - strlen(".");
158+
len += strlen(decimal_point) - 1;
161159

162160
return len;
163161
}
164162

165-
static int
166-
strlen_with_numeric_locale(const char *my_str)
167-
{
168-
return strlen(my_str) + additional_numeric_locale_len(my_str);
169-
}
170-
171163
/*
172164
* Returns the appropriately formatted string in a new allocated block,
173165
* caller must free
174166
*/
175167
static char *
176168
format_numeric_locale(const char *my_str)
177169
{
170+
int new_len = strlen(my_str) + additional_numeric_locale_len(my_str);
171+
char *new_str = pg_malloc(new_len + 1);
172+
int int_len = integer_digits(my_str);
178173
int i,
179-
j,
180-
int_len = integer_digits(my_str),
181174
leading_digits;
182-
int groupdigits = atoi(grouping);
183-
int new_str_start = 0;
184-
char *new_str = pg_malloc(strlen_with_numeric_locale(my_str) + 1);
175+
int new_str_pos = 0;
185176

186-
leading_digits = (int_len % groupdigits != 0) ?
187-
int_len % groupdigits : groupdigits;
177+
/* number of digits in first thousands group */
178+
leading_digits = int_len % groupdigits;
179+
if (leading_digits == 0)
180+
leading_digits = groupdigits;
188181

189-
if (my_str[0] == '-') /* skip over sign, affects grouping
190-
* calculations */
182+
/* process sign */
183+
if (my_str[0] == '-' || my_str[0] == '+')
191184
{
192-
new_str[0] = my_str[0];
185+
new_str[new_str_pos++] = my_str[0];
193186
my_str++;
194-
new_str_start = 1;
195187
}
196188

197-
for (i = 0, j = new_str_start;; i++, j++)
189+
/* process integer part of number */
190+
for (i = 0; i < int_len; i++)
198191
{
199-
/* Hit decimal point? */
200-
if (my_str[i] == '.')
192+
/* Time to insert separator? */
193+
if (i > 0 && --leading_digits == 0)
201194
{
202-
strcpy(&new_str[j], decimal_point);
203-
j += strlen(decimal_point);
204-
/* add fractional part */
205-
strcpy(&new_str[j], &my_str[i] + 1);
206-
break;
207-
}
208-
209-
/* End of string? */
210-
if (my_str[i] == '\0')
211-
{
212-
new_str[j] = '\0';
213-
break;
214-
}
215-
216-
/* Add separator? */
217-
if (i != 0 && (i - leading_digits) % groupdigits == 0)
218-
{
219-
strcpy(&new_str[j], thousands_sep);
220-
j += strlen(thousands_sep);
195+
strcpy(&new_str[new_str_pos], thousands_sep);
196+
new_str_pos += strlen(thousands_sep);
197+
leading_digits = groupdigits;
221198
}
199+
new_str[new_str_pos++] = my_str[i];
200+
}
222201

223-
new_str[j] = my_str[i];
202+
/* handle decimal point if any */
203+
if (my_str[i] == '.')
204+
{
205+
strcpy(&new_str[new_str_pos], decimal_point);
206+
new_str_pos += strlen(decimal_point);
207+
i++;
224208
}
225209

210+
/* copy the rest (fractional digits and/or exponent, and \0 terminator) */
211+
strcpy(&new_str[new_str_pos], &my_str[i]);
212+
226213
return new_str;
227214
}
228215

@@ -2674,10 +2661,11 @@ setDecimalLocale(void)
26742661
decimal_point = pg_strdup(extlconv->decimal_point);
26752662
else
26762663
decimal_point = "."; /* SQL output standard */
2664+
26772665
if (*extlconv->grouping && atoi(extlconv->grouping) > 0)
2678-
grouping = pg_strdup(extlconv->grouping);
2666+
groupdigits = atoi(extlconv->grouping);
26792667
else
2680-
grouping = "3"; /* most common */
2668+
groupdigits = 3; /* most common */
26812669

26822670
/* similar code exists in formatting.c */
26832671
if (*extlconv->thousands_sep)

0 commit comments

Comments
 (0)