Skip to content

Commit 49917ed

Browse files
committed
Further fix for psql's code for locale-aware formatting of numeric output.
On closer inspection, those seemingly redundant atoi() calls were not so much inefficient as just plain wrong: the author of this code either had not read, or had not understood, the POSIX specification for localeconv(). The grouping field is *not* a textual digit string but separate integers encoded as chars. We'll follow the existing code as well as the backend's cash.c in only honoring the first group width, but let's at least honor it correctly. This doesn't actually result in any behavioral change in any of the locales I have installed on my Linux box, which may explain why nobody's complained; grouping width 3 is close enough to universal that it's barely worth considering other cases. Still, wrong is wrong, so back-patch.
1 parent 348dd28 commit 49917ed

File tree

1 file changed

+11
-3
lines changed

1 file changed

+11
-3
lines changed

src/bin/psql/print.c

+11-3
Original file line numberDiff line numberDiff line change
@@ -2664,16 +2664,24 @@ setDecimalLocale(void)
26642664

26652665
extlconv = localeconv();
26662666

2667+
/* Don't accept an empty decimal_point string */
26672668
if (*extlconv->decimal_point)
26682669
decimal_point = pg_strdup(extlconv->decimal_point);
26692670
else
26702671
decimal_point = "."; /* SQL output standard */
26712672

2672-
if (*extlconv->grouping && atoi(extlconv->grouping) > 0)
2673-
groupdigits = atoi(extlconv->grouping);
2674-
else
2673+
/*
2674+
* Although the Open Group standard allows locales to supply more than one
2675+
* group width, we consider only the first one, and we ignore any attempt
2676+
* to suppress grouping by specifying CHAR_MAX. As in the backend's
2677+
* cash.c, we must apply a range check to avoid being fooled by variant
2678+
* CHAR_MAX values.
2679+
*/
2680+
groupdigits = *extlconv->grouping;
2681+
if (groupdigits <= 0 || groupdigits > 6)
26752682
groupdigits = 3; /* most common */
26762683

2684+
/* Don't accept an empty thousands_sep string, either */
26772685
/* similar code exists in formatting.c */
26782686
if (*extlconv->thousands_sep)
26792687
thousands_sep = pg_strdup(extlconv->thousands_sep);

0 commit comments

Comments
 (0)