Skip to content

Commit 147bbc9

Browse files
tglsfdctglsfdc
committed
Modernize to_char's Roman-numeral code, fixing overflow problems.
int_to_roman() only accepts plain "int" input, which is fine since we're going to produce '###############' for any value above 3999 anyway. However, the numeric and int8 variants of to_char() would throw an error if the given input exceeded the integer range, while the float-input variants invoked undefined-per-C-standard behavior. Fix things so that you uniformly get '###############' for out of range input. Also add test cases covering this code, plus the equally-untested EEEE, V, and PL format codes. Discussion: https://postgr.es/m/2956175.1725831136@sss.pgh.pa.us
1 parent e3a92ab commit 147bbc9

File tree

6 files changed

+231
-23
lines changed

6 files changed

+231
-23
lines changed

‎doc/src/sgml/func.sgml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8739,6 +8739,8 @@ SELECT regexp_match('abc01234xyz', '(?:(.*?)(\d+)(.*)){1,1}');
87398739
<replaceable>n</replaceable> is the number of digits following
87408740
<literal>V</literal>. <literal>V</literal> with
87418741
<function>to_number</function> divides in a similar manner.
8742+
The <literal>V</literal> can be thought of as marking the position
8743+
of an implicit decimal point in the input or output string.
87428744
<function>to_char</function> and <function>to_number</function>
87438745
do not support the use of
87448746
<literal>V</literal> combined with a decimal point

‎src/backend/utils/adt/formatting.c‎

Lines changed: 69 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5189,6 +5189,11 @@ NUM_cache(int len, NUMDesc *Num, text *pars_str, bool *shouldFree)
51895189
}
51905190

51915191

5192+
/*
5193+
* Convert integer to Roman numerals
5194+
* Result is upper-case and not blank-padded (NUM_processor converts as needed)
5195+
* If input is out-of-range, produce '###############'
5196+
*/
51925197
static char *
51935198
int_to_roman(int number)
51945199
{
@@ -5201,32 +5206,42 @@ int_to_roman(int number)
52015206
result = (char *) palloc(16);
52025207
*result = '\0';
52035208

5209+
/*
5210+
* This range limit is the same as in Oracle(TM). The difficulty with
5211+
* handling 4000 or more is that we'd need to use more than 3 "M"'s, and
5212+
* more than 3 of the same digit isn't considered a valid Roman string.
5213+
*/
52045214
if (number > 3999 || number < 1)
52055215
{
52065216
fill_str(result, '#', 15);
52075217
return result;
52085218
}
5219+
5220+
/* Convert to decimal, then examine each digit */
52095221
len = snprintf(numstr, sizeof(numstr), "%d", number);
5222+
Assert(len > 0 && len <= 4);
52105223

52115224
for (p = numstr; *p != '\0'; p++, --len)
52125225
{
52135226
num = *p - ('0' + 1);
52145227
if (num < 0)
5215-
continue;
5216-
5217-
if (len > 3)
5228+
continue; /* ignore zeroes */
5229+
/* switch on current column position */
5230+
switch (len)
52185231
{
5219-
while (num-- != -1)
5220-
strcat(result, "M");
5221-
}
5222-
else
5223-
{
5224-
if (len == 3)
5232+
case 4:
5233+
while (num-- >= 0)
5234+
strcat(result, "M");
5235+
break;
5236+
case 3:
52255237
strcat(result, rm100[num]);
5226-
else if (len == 2)
5238+
break;
5239+
case 2:
52275240
strcat(result, rm10[num]);
5228-
else if (len == 1)
5241+
break;
5242+
case 1:
52295243
strcat(result, rm1[num]);
5244+
break;
52305245
}
52315246
}
52325247
return result;
@@ -6367,7 +6382,6 @@ numeric_to_char(PG_FUNCTION_ARGS)
63676382
char *numstr,
63686383
*orgnum,
63696384
*p;
6370-
Numeric x;
63716385

63726386
NUM_TOCHAR_prepare;
63736387

@@ -6376,12 +6390,15 @@ numeric_to_char(PG_FUNCTION_ARGS)
63766390
*/
63776391
if (IS_ROMAN(&Num))
63786392
{
6379-
x = DatumGetNumeric(DirectFunctionCall2(numeric_round,
6380-
NumericGetDatum(value),
6381-
Int32GetDatum(0)));
6382-
numstr =
6383-
int_to_roman(DatumGetInt32(DirectFunctionCall1(numeric_int4,
6384-
NumericGetDatum(x))));
6393+
int32 intvalue;
6394+
bool err;
6395+
6396+
/* Round and convert to int */
6397+
intvalue = numeric_int4_opt_error(value, &err);
6398+
/* On overflow, just use PG_INT32_MAX; int_to_roman will cope */
6399+
if (err)
6400+
intvalue = PG_INT32_MAX;
6401+
numstr = int_to_roman(intvalue);
63856402
}
63866403
else if (IS_EEEE(&Num))
63876404
{
@@ -6421,6 +6438,7 @@ numeric_to_char(PG_FUNCTION_ARGS)
64216438
{
64226439
int numstr_pre_len;
64236440
Numeric val = value;
6441+
Numeric x;
64246442

64256443
if (IS_MULTI(&Num))
64266444
{
@@ -6589,12 +6607,18 @@ int8_to_char(PG_FUNCTION_ARGS)
65896607
NUM_TOCHAR_prepare;
65906608

65916609
/*
6592-
* On DateType depend part (int32)
6610+
* On DateType depend part (int64)
65936611
*/
65946612
if (IS_ROMAN(&Num))
65956613
{
6596-
/* Currently don't support int8 conversion to roman... */
6597-
numstr = int_to_roman(DatumGetInt32(DirectFunctionCall1(int84, Int64GetDatum(value))));
6614+
int32 intvalue;
6615+
6616+
/* On overflow, just use PG_INT32_MAX; int_to_roman will cope */
6617+
if (value <= PG_INT32_MAX && value >= PG_INT32_MIN)
6618+
intvalue = (int32) value;
6619+
else
6620+
intvalue = PG_INT32_MAX;
6621+
numstr = int_to_roman(intvalue);
65986622
}
65996623
else if (IS_EEEE(&Num))
66006624
{
@@ -6695,7 +6719,18 @@ float4_to_char(PG_FUNCTION_ARGS)
66956719
NUM_TOCHAR_prepare;
66966720

66976721
if (IS_ROMAN(&Num))
6698-
numstr = int_to_roman((int) rint(value));
6722+
{
6723+
int32 intvalue;
6724+
6725+
/* See notes in ftoi4() */
6726+
value = rint(value);
6727+
/* On overflow, just use PG_INT32_MAX; int_to_roman will cope */
6728+
if (!isnan(value) && FLOAT4_FITS_IN_INT32(value))
6729+
intvalue = (int32) value;
6730+
else
6731+
intvalue = PG_INT32_MAX;
6732+
numstr = int_to_roman(intvalue);
6733+
}
66996734
else if (IS_EEEE(&Num))
67006735
{
67016736
if (isnan(value) || isinf(value))
@@ -6797,7 +6832,18 @@ float8_to_char(PG_FUNCTION_ARGS)
67976832
NUM_TOCHAR_prepare;
67986833

67996834
if (IS_ROMAN(&Num))
6800-
numstr = int_to_roman((int) rint(value));
6835+
{
6836+
int32 intvalue;
6837+
6838+
/* See notes in dtoi4() */
6839+
value = rint(value);
6840+
/* On overflow, just use PG_INT32_MAX; int_to_roman will cope */
6841+
if (!isnan(value) && FLOAT8_FITS_IN_INT32(value))
6842+
intvalue = (int32) value;
6843+
else
6844+
intvalue = PG_INT32_MAX;
6845+
numstr = int_to_roman(intvalue);
6846+
}
68016847
else if (IS_EEEE(&Num))
68026848
{
68036849
if (isnan(value) || isinf(value))

‎src/test/regress/expected/int8.out‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,16 @@ SELECT to_char(q2, 'MI9999999999999999') FROM INT8_TBL;
530530
-4567890123456789
531531
(5 rows)
532532

533+
SELECT to_char(q2, '9999999999999999PL') FROM INT8_TBL;
534+
to_char
535+
--------------------
536+
456+
537+
4567890123456789+
538+
123+
539+
4567890123456789+
540+
-4567890123456789
541+
(5 rows)
542+
533543
SELECT to_char(q2, 'FMS9999999999999999') FROM INT8_TBL;
534544
to_char
535545
-------------------
@@ -650,6 +660,46 @@ SELECT to_char(q2, '999999SG9999999999') FROM INT8_TBL;
650660
456789-0123456789
651661
(5 rows)
652662

663+
SELECT to_char(q2, 'FMRN') FROM INT8_TBL;
664+
to_char
665+
-----------------
666+
CDLVI
667+
###############
668+
CXXIII
669+
###############
670+
###############
671+
(5 rows)
672+
673+
SELECT to_char(1234, '9.99EEEE');
674+
to_char
675+
-----------
676+
1.23e+03
677+
(1 row)
678+
679+
SELECT to_char(1234::int8, '9.99eeee');
680+
to_char
681+
-----------
682+
1.23e+03
683+
(1 row)
684+
685+
SELECT to_char(-1234::int8, '9.99eeee');
686+
to_char
687+
-----------
688+
-1.23e+03
689+
(1 row)
690+
691+
SELECT to_char(1234, '99999V99');
692+
to_char
693+
----------
694+
123400
695+
(1 row)
696+
697+
SELECT to_char(1234::int8, '99999V99');
698+
to_char
699+
----------
700+
123400
701+
(1 row)
702+
653703
-- check min/max values and overflow behavior
654704
select '-9223372036854775808'::int8;
655705
int8

‎src/test/regress/expected/numeric.out‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1991,6 +1991,21 @@ SELECT to_char(val, '9.999EEEE') FROM num_data;
19911991
-2.493e+07
19921992
(10 rows)
19931993

1994+
SELECT to_char(val, 'FMRN') FROM num_data;
1995+
to_char
1996+
-----------------
1997+
###############
1998+
###############
1999+
###############
2000+
IV
2001+
###############
2002+
###############
2003+
###############
2004+
###############
2005+
###############
2006+
###############
2007+
(10 rows)
2008+
19942009
WITH v(val) AS
19952010
(VALUES('0'::numeric),('-4.2'),('4.2e9'),('1.2e-5'),('inf'),('-inf'),('nan'))
19962011
SELECT val,
@@ -2101,6 +2116,72 @@ SELECT to_char('12345678901'::float8, 'FM9999999999D9999900000000000000000');
21012116
##########.####
21022117
(1 row)
21032118

2119+
SELECT to_char('100'::numeric, 'rn');
2120+
to_char
2121+
-----------------
2122+
c
2123+
(1 row)
2124+
2125+
SELECT to_char('1234'::numeric, 'rn');
2126+
to_char
2127+
-----------------
2128+
mccxxxiv
2129+
(1 row)
2130+
2131+
SELECT to_char('1235'::float4, 'rn');
2132+
to_char
2133+
-----------------
2134+
mccxxxv
2135+
(1 row)
2136+
2137+
SELECT to_char('1236'::float8, 'rn');
2138+
to_char
2139+
-----------------
2140+
mccxxxvi
2141+
(1 row)
2142+
2143+
SELECT to_char('1237'::float8, 'fmrn');
2144+
to_char
2145+
-----------
2146+
mccxxxvii
2147+
(1 row)
2148+
2149+
SELECT to_char('100e9'::numeric, 'RN');
2150+
to_char
2151+
-----------------
2152+
###############
2153+
(1 row)
2154+
2155+
SELECT to_char('100e9'::float4, 'RN');
2156+
to_char
2157+
-----------------
2158+
###############
2159+
(1 row)
2160+
2161+
SELECT to_char('100e9'::float8, 'RN');
2162+
to_char
2163+
-----------------
2164+
###############
2165+
(1 row)
2166+
2167+
SELECT to_char(1234.56::numeric, '99999V99');
2168+
to_char
2169+
----------
2170+
123456
2171+
(1 row)
2172+
2173+
SELECT to_char(1234.56::float4, '99999V99');
2174+
to_char
2175+
----------
2176+
123456
2177+
(1 row)
2178+
2179+
SELECT to_char(1234.56::float8, '99999V99');
2180+
to_char
2181+
----------
2182+
123456
2183+
(1 row)
2184+
21042185
-- Check parsing of literal text in a format string
21052186
SELECT to_char('100'::numeric, 'foo999');
21062187
to_char
@@ -2297,6 +2378,12 @@ SELECT to_number('42nd', '99th');
22972378
42
22982379
(1 row)
22992380

2381+
SELECT to_number('123456', '99999V99');
2382+
to_number
2383+
-------------------------
2384+
1234.560000000000000000
2385+
(1 row)
2386+
23002387
RESET lc_numeric;
23012388
--
23022389
-- Input syntax

‎src/test/regress/sql/int8.sql‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ SELECT to_char( (q1 * -1), '9999999999999999S'), to_char( (q2 * -1), 'S999999999
110110
FROM INT8_TBL;
111111

112112
SELECT to_char(q2, 'MI9999999999999999') FROM INT8_TBL;
113+
SELECT to_char(q2, '9999999999999999PL') FROM INT8_TBL;
113114
SELECT to_char(q2, 'FMS9999999999999999') FROM INT8_TBL;
114115
SELECT to_char(q2, 'FM9999999999999999THPR') FROM INT8_TBL;
115116
SELECT to_char(q2, 'SG9999999999999999th') FROM INT8_TBL;
@@ -122,6 +123,13 @@ SELECT to_char(q2, 'FM9999999999999999.999') FROM INT8_TBL;
122123
SELECT to_char(q2, 'S 9 9 9 9 9 9 9 9 9 9 9 9 9 9 9 9 . 9 9 9') FROM INT8_TBL;
123124
SELECT to_char(q2, E'99999 "text" 9999 "9999" 999 "\\"text between quote marks\\"" 9999') FROM INT8_TBL;
124125
SELECT to_char(q2, '999999SG9999999999') FROM INT8_TBL;
126+
SELECT to_char(q2, 'FMRN') FROM INT8_TBL;
127+
128+
SELECT to_char(1234, '9.99EEEE');
129+
SELECT to_char(1234::int8, '9.99eeee');
130+
SELECT to_char(-1234::int8, '9.99eeee');
131+
SELECT to_char(1234, '99999V99');
132+
SELECT to_char(1234::int8, '99999V99');
125133

126134
-- check min/max values and overflow behavior
127135

0 commit comments

Comments
 (0)