Thread: Test improvements and minor code fixes for formatting.c.
Over in the thread about teaching to_number() to handle Roman
numerals [1], it was noted that we currently have precisely zero
test coverage for to_char()'s existing Roman-numeral code, except
in the timestamp code path which shares nothing with the numeric
code path.
In looking at this, I found that there's also no test coverage
for the EEEE, V, or PL format codes. Also, the possibility of
overflow while converting an input value to int in order to
pass it to int_to_roman was ignored. Attached is a patch that
adds more test coverage and cleans up the Roman-numeral code
a little bit.
BTW, I also discovered that there is a little bit of support
for a "B" format code: we can parse it, but then we ignore it.
And it's not documented. Oracle defines this code as a flag
that:
Returns blanks for the integer part of a fixed-point number
when the integer part is zero (regardless of zeros in the
format model).
It doesn't seem super hard to implement that, but given the
complete lack of complaints about it being missing, maybe we
should just rip out the incomplete support instead?
regards, tom lane
[1] https://www.postgresql.org/message-id/flat/CAMWA6ybh4M1VQqpmnu2tfSwO%2B3gAPeA8YKnMHVADeB%3DXDEvT_A%40mail.gmail.com
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 461fc3f437..e2d45989d8 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -8739,6 +8739,8 @@ SELECT regexp_match('abc01234xyz', '(?:(.*?)(\d+)(.*)){1,1}');
<replaceable>n</replaceable> is the number of digits following
<literal>V</literal>. <literal>V</literal> with
<function>to_number</function> divides in a similar manner.
+ The <literal>V</literal> can be thought of as marking the position
+ of an implicit decimal point in the input or output string.
<function>to_char</function> and <function>to_number</function>
do not support the use of
<literal>V</literal> combined with a decimal point
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 68fa89418f..116d79ae11 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -5189,6 +5189,11 @@ NUM_cache(int len, NUMDesc *Num, text *pars_str, bool *shouldFree)
}
+/*
+ * Convert integer to Roman numerals
+ * Result is upper-case and not blank-padded (NUM_processor converts as needed)
+ * If input is out-of-range, produce '###############'
+ */
static char *
int_to_roman(int number)
{
@@ -5201,32 +5206,42 @@ int_to_roman(int number)
result = (char *) palloc(16);
*result = '\0';
+ /*
+ * This range limit is the same as in Oracle(TM). The difficulty with
+ * handling 4000 or more is that we'd need to use more than 3 "M"'s, and
+ * more than 3 of the same digit isn't considered a valid Roman string.
+ */
if (number > 3999 || number < 1)
{
fill_str(result, '#', 15);
return result;
}
+
+ /* Convert to decimal, then examine each digit */
len = snprintf(numstr, sizeof(numstr), "%d", number);
+ Assert(len > 0 && len <= 4);
for (p = numstr; *p != '\0'; p++, --len)
{
num = *p - ('0' + 1);
if (num < 0)
- continue;
-
- if (len > 3)
+ continue; /* ignore zeroes */
+ /* switch on current column position */
+ switch (len)
{
- while (num-- != -1)
- strcat(result, "M");
- }
- else
- {
- if (len == 3)
+ case 4:
+ while (num-- >= 0)
+ strcat(result, "M");
+ break;
+ case 3:
strcat(result, rm100[num]);
- else if (len == 2)
+ break;
+ case 2:
strcat(result, rm10[num]);
- else if (len == 1)
+ break;
+ case 1:
strcat(result, rm1[num]);
+ break;
}
}
return result;
@@ -6367,7 +6382,6 @@ numeric_to_char(PG_FUNCTION_ARGS)
char *numstr,
*orgnum,
*p;
- Numeric x;
NUM_TOCHAR_prepare;
@@ -6376,12 +6390,15 @@ numeric_to_char(PG_FUNCTION_ARGS)
*/
if (IS_ROMAN(&Num))
{
- x = DatumGetNumeric(DirectFunctionCall2(numeric_round,
- NumericGetDatum(value),
- Int32GetDatum(0)));
- numstr =
- int_to_roman(DatumGetInt32(DirectFunctionCall1(numeric_int4,
- NumericGetDatum(x))));
+ int32 intvalue;
+ bool err;
+
+ /* Round and convert to int */
+ intvalue = numeric_int4_opt_error(value, &err);
+ /* On overflow, just use PG_INT32_MAX; int_to_roman will cope */
+ if (err)
+ intvalue = PG_INT32_MAX;
+ numstr = int_to_roman(intvalue);
}
else if (IS_EEEE(&Num))
{
@@ -6421,6 +6438,7 @@ numeric_to_char(PG_FUNCTION_ARGS)
{
int numstr_pre_len;
Numeric val = value;
+ Numeric x;
if (IS_MULTI(&Num))
{
@@ -6589,12 +6607,18 @@ int8_to_char(PG_FUNCTION_ARGS)
NUM_TOCHAR_prepare;
/*
- * On DateType depend part (int32)
+ * On DateType depend part (int64)
*/
if (IS_ROMAN(&Num))
{
- /* Currently don't support int8 conversion to roman... */
- numstr = int_to_roman(DatumGetInt32(DirectFunctionCall1(int84, Int64GetDatum(value))));
+ int32 intvalue;
+
+ /* On overflow, just use PG_INT32_MAX; int_to_roman will cope */
+ if (value <= PG_INT32_MAX && value >= PG_INT32_MIN)
+ intvalue = (int32) value;
+ else
+ intvalue = PG_INT32_MAX;
+ numstr = int_to_roman(intvalue);
}
else if (IS_EEEE(&Num))
{
@@ -6695,7 +6719,16 @@ float4_to_char(PG_FUNCTION_ARGS)
NUM_TOCHAR_prepare;
if (IS_ROMAN(&Num))
- numstr = int_to_roman((int) rint(value));
+ {
+ int32 intvalue;
+
+ /* On overflow, just use PG_INT32_MAX; int_to_roman will cope */
+ if (!isnan(value) && value <= PG_INT32_MAX && value >= PG_INT32_MIN)
+ intvalue = (int32) rint(value);
+ else
+ intvalue = PG_INT32_MAX;
+ numstr = int_to_roman(intvalue);
+ }
else if (IS_EEEE(&Num))
{
if (isnan(value) || isinf(value))
@@ -6797,7 +6830,16 @@ float8_to_char(PG_FUNCTION_ARGS)
NUM_TOCHAR_prepare;
if (IS_ROMAN(&Num))
- numstr = int_to_roman((int) rint(value));
+ {
+ int32 intvalue;
+
+ /* On overflow, just use PG_INT32_MAX; int_to_roman will cope */
+ if (!isnan(value) && value <= PG_INT32_MAX && value >= PG_INT32_MIN)
+ intvalue = (int32) rint(value);
+ else
+ intvalue = PG_INT32_MAX;
+ numstr = int_to_roman(intvalue);
+ }
else if (IS_EEEE(&Num))
{
if (isnan(value) || isinf(value))
diff --git a/src/test/regress/expected/int8.out b/src/test/regress/expected/int8.out
index fddc09f630..392f3bf655 100644
--- a/src/test/regress/expected/int8.out
+++ b/src/test/regress/expected/int8.out
@@ -530,6 +530,16 @@ SELECT to_char(q2, 'MI9999999999999999') FROM INT8_TBL;
-4567890123456789
(5 rows)
+SELECT to_char(q2, '9999999999999999PL') FROM INT8_TBL;
+ to_char
+--------------------
+ 456+
+ 4567890123456789+
+ 123+
+ 4567890123456789+
+ -4567890123456789
+(5 rows)
+
SELECT to_char(q2, 'FMS9999999999999999') FROM INT8_TBL;
to_char
-------------------
@@ -650,6 +660,46 @@ SELECT to_char(q2, '999999SG9999999999') FROM INT8_TBL;
456789-0123456789
(5 rows)
+SELECT to_char(q2, 'FMRN') FROM INT8_TBL;
+ to_char
+-----------------
+ CDLVI
+ ###############
+ CXXIII
+ ###############
+ ###############
+(5 rows)
+
+SELECT to_char(1234, '9.99EEEE');
+ to_char
+-----------
+ 1.23e+03
+(1 row)
+
+SELECT to_char(1234::int8, '9.99eeee');
+ to_char
+-----------
+ 1.23e+03
+(1 row)
+
+SELECT to_char(-1234::int8, '9.99eeee');
+ to_char
+-----------
+ -1.23e+03
+(1 row)
+
+SELECT to_char(1234, '99999V99');
+ to_char
+----------
+ 123400
+(1 row)
+
+SELECT to_char(1234::int8, '99999V99');
+ to_char
+----------
+ 123400
+(1 row)
+
-- check min/max values and overflow behavior
select '-9223372036854775808'::int8;
int8
diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out
index f30ac236f5..d54282496a 100644
--- a/src/test/regress/expected/numeric.out
+++ b/src/test/regress/expected/numeric.out
@@ -1991,6 +1991,21 @@ SELECT to_char(val, '9.999EEEE') FROM num_data;
-2.493e+07
(10 rows)
+SELECT to_char(val, 'FMRN') FROM num_data;
+ to_char
+-----------------
+ ###############
+ ###############
+ ###############
+ IV
+ ###############
+ ###############
+ ###############
+ ###############
+ ###############
+ ###############
+(10 rows)
+
WITH v(val) AS
(VALUES('0'::numeric),('-4.2'),('4.2e9'),('1.2e-5'),('inf'),('-inf'),('nan'))
SELECT val,
@@ -2101,6 +2116,72 @@ SELECT to_char('12345678901'::float8, 'FM9999999999D9999900000000000000000');
##########.####
(1 row)
+SELECT to_char('100'::numeric, 'rn');
+ to_char
+-----------------
+ c
+(1 row)
+
+SELECT to_char('1234'::numeric, 'rn');
+ to_char
+-----------------
+ mccxxxiv
+(1 row)
+
+SELECT to_char('1235'::float4, 'rn');
+ to_char
+-----------------
+ mccxxxv
+(1 row)
+
+SELECT to_char('1236'::float8, 'rn');
+ to_char
+-----------------
+ mccxxxvi
+(1 row)
+
+SELECT to_char('1237'::float8, 'fmrn');
+ to_char
+-----------
+ mccxxxvii
+(1 row)
+
+SELECT to_char('100e9'::numeric, 'RN');
+ to_char
+-----------------
+ ###############
+(1 row)
+
+SELECT to_char('100e9'::float4, 'RN');
+ to_char
+-----------------
+ ###############
+(1 row)
+
+SELECT to_char('100e9'::float8, 'RN');
+ to_char
+-----------------
+ ###############
+(1 row)
+
+SELECT to_char(1234.56::numeric, '99999V99');
+ to_char
+----------
+ 123456
+(1 row)
+
+SELECT to_char(1234.56::float4, '99999V99');
+ to_char
+----------
+ 123456
+(1 row)
+
+SELECT to_char(1234.56::float8, '99999V99');
+ to_char
+----------
+ 123456
+(1 row)
+
-- Check parsing of literal text in a format string
SELECT to_char('100'::numeric, 'foo999');
to_char
@@ -2297,6 +2378,12 @@ SELECT to_number('42nd', '99th');
42
(1 row)
+SELECT to_number('123456', '99999V99');
+ to_number
+-------------------------
+ 1234.560000000000000000
+(1 row)
+
RESET lc_numeric;
--
-- Input syntax
diff --git a/src/test/regress/sql/int8.sql b/src/test/regress/sql/int8.sql
index fffb28906a..53359f06ad 100644
--- a/src/test/regress/sql/int8.sql
+++ b/src/test/regress/sql/int8.sql
@@ -110,6 +110,7 @@ SELECT to_char( (q1 * -1), '9999999999999999S'), to_char( (q2 * -1), 'S999999999
FROM INT8_TBL;
SELECT to_char(q2, 'MI9999999999999999') FROM INT8_TBL;
+SELECT to_char(q2, '9999999999999999PL') FROM INT8_TBL;
SELECT to_char(q2, 'FMS9999999999999999') FROM INT8_TBL;
SELECT to_char(q2, 'FM9999999999999999THPR') FROM INT8_TBL;
SELECT to_char(q2, 'SG9999999999999999th') FROM INT8_TBL;
@@ -122,6 +123,13 @@ SELECT to_char(q2, 'FM9999999999999999.999') FROM INT8_TBL;
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;
SELECT to_char(q2, E'99999 "text" 9999 "9999" 999 "\\"text between quote marks\\"" 9999') FROM INT8_TBL;
SELECT to_char(q2, '999999SG9999999999') FROM INT8_TBL;
+SELECT to_char(q2, 'FMRN') FROM INT8_TBL;
+
+SELECT to_char(1234, '9.99EEEE');
+SELECT to_char(1234::int8, '9.99eeee');
+SELECT to_char(-1234::int8, '9.99eeee');
+SELECT to_char(1234, '99999V99');
+SELECT to_char(1234::int8, '99999V99');
-- check min/max values and overflow behavior
diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql
index c86395209a..b508cba71d 100644
--- a/src/test/regress/sql/numeric.sql
+++ b/src/test/regress/sql/numeric.sql
@@ -996,6 +996,7 @@ SELECT to_char(val, E'99999 "text" 9999 "9999" 999 "\\"text between quote marks\
SELECT to_char(val, '999999SG9999999999') FROM num_data;
SELECT to_char(val, 'FM9999999999999999.999999999999999') FROM num_data;
SELECT to_char(val, '9.999EEEE') FROM num_data;
+SELECT to_char(val, 'FMRN') FROM num_data;
WITH v(val) AS
(VALUES('0'::numeric),('-4.2'),('4.2e9'),('1.2e-5'),('inf'),('-inf'),('nan'))
@@ -1033,6 +1034,19 @@ SELECT to_char('100'::numeric, 'FM999.');
SELECT to_char('100'::numeric, 'FM999');
SELECT to_char('12345678901'::float8, 'FM9999999999D9999900000000000000000');
+SELECT to_char('100'::numeric, 'rn');
+SELECT to_char('1234'::numeric, 'rn');
+SELECT to_char('1235'::float4, 'rn');
+SELECT to_char('1236'::float8, 'rn');
+SELECT to_char('1237'::float8, 'fmrn');
+SELECT to_char('100e9'::numeric, 'RN');
+SELECT to_char('100e9'::float4, 'RN');
+SELECT to_char('100e9'::float8, 'RN');
+
+SELECT to_char(1234.56::numeric, '99999V99');
+SELECT to_char(1234.56::float4, '99999V99');
+SELECT to_char(1234.56::float8, '99999V99');
+
-- Check parsing of literal text in a format string
SELECT to_char('100'::numeric, 'foo999');
SELECT to_char('100'::numeric, 'f\oo999');
@@ -1070,6 +1084,7 @@ SELECT to_number('$1,234.56','L99,999.99');
SELECT to_number('1234.56','L99,999.99');
SELECT to_number('1,234.56','L99,999.99');
SELECT to_number('42nd', '99th');
+SELECT to_number('123456', '99999V99');
RESET lc_numeric;
--
I wrote:
> [ v1-formatting-test-improvements.patch ]
Meh ... cfbot points out I did the float-to-int conversions wrong.
v2 attached.
regards, tom lane
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 461fc3f437..e2d45989d8 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -8739,6 +8739,8 @@ SELECT regexp_match('abc01234xyz', '(?:(.*?)(\d+)(.*)){1,1}');
<replaceable>n</replaceable> is the number of digits following
<literal>V</literal>. <literal>V</literal> with
<function>to_number</function> divides in a similar manner.
+ The <literal>V</literal> can be thought of as marking the position
+ of an implicit decimal point in the input or output string.
<function>to_char</function> and <function>to_number</function>
do not support the use of
<literal>V</literal> combined with a decimal point
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 68fa89418f..85a7dd4561 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -5189,6 +5189,11 @@ NUM_cache(int len, NUMDesc *Num, text *pars_str, bool *shouldFree)
}
+/*
+ * Convert integer to Roman numerals
+ * Result is upper-case and not blank-padded (NUM_processor converts as needed)
+ * If input is out-of-range, produce '###############'
+ */
static char *
int_to_roman(int number)
{
@@ -5201,32 +5206,42 @@ int_to_roman(int number)
result = (char *) palloc(16);
*result = '\0';
+ /*
+ * This range limit is the same as in Oracle(TM). The difficulty with
+ * handling 4000 or more is that we'd need to use more than 3 "M"'s, and
+ * more than 3 of the same digit isn't considered a valid Roman string.
+ */
if (number > 3999 || number < 1)
{
fill_str(result, '#', 15);
return result;
}
+
+ /* Convert to decimal, then examine each digit */
len = snprintf(numstr, sizeof(numstr), "%d", number);
+ Assert(len > 0 && len <= 4);
for (p = numstr; *p != '\0'; p++, --len)
{
num = *p - ('0' + 1);
if (num < 0)
- continue;
-
- if (len > 3)
+ continue; /* ignore zeroes */
+ /* switch on current column position */
+ switch (len)
{
- while (num-- != -1)
- strcat(result, "M");
- }
- else
- {
- if (len == 3)
+ case 4:
+ while (num-- >= 0)
+ strcat(result, "M");
+ break;
+ case 3:
strcat(result, rm100[num]);
- else if (len == 2)
+ break;
+ case 2:
strcat(result, rm10[num]);
- else if (len == 1)
+ break;
+ case 1:
strcat(result, rm1[num]);
+ break;
}
}
return result;
@@ -6367,7 +6382,6 @@ numeric_to_char(PG_FUNCTION_ARGS)
char *numstr,
*orgnum,
*p;
- Numeric x;
NUM_TOCHAR_prepare;
@@ -6376,12 +6390,15 @@ numeric_to_char(PG_FUNCTION_ARGS)
*/
if (IS_ROMAN(&Num))
{
- x = DatumGetNumeric(DirectFunctionCall2(numeric_round,
- NumericGetDatum(value),
- Int32GetDatum(0)));
- numstr =
- int_to_roman(DatumGetInt32(DirectFunctionCall1(numeric_int4,
- NumericGetDatum(x))));
+ int32 intvalue;
+ bool err;
+
+ /* Round and convert to int */
+ intvalue = numeric_int4_opt_error(value, &err);
+ /* On overflow, just use PG_INT32_MAX; int_to_roman will cope */
+ if (err)
+ intvalue = PG_INT32_MAX;
+ numstr = int_to_roman(intvalue);
}
else if (IS_EEEE(&Num))
{
@@ -6421,6 +6438,7 @@ numeric_to_char(PG_FUNCTION_ARGS)
{
int numstr_pre_len;
Numeric val = value;
+ Numeric x;
if (IS_MULTI(&Num))
{
@@ -6589,12 +6607,18 @@ int8_to_char(PG_FUNCTION_ARGS)
NUM_TOCHAR_prepare;
/*
- * On DateType depend part (int32)
+ * On DateType depend part (int64)
*/
if (IS_ROMAN(&Num))
{
- /* Currently don't support int8 conversion to roman... */
- numstr = int_to_roman(DatumGetInt32(DirectFunctionCall1(int84, Int64GetDatum(value))));
+ int32 intvalue;
+
+ /* On overflow, just use PG_INT32_MAX; int_to_roman will cope */
+ if (value <= PG_INT32_MAX && value >= PG_INT32_MIN)
+ intvalue = (int32) value;
+ else
+ intvalue = PG_INT32_MAX;
+ numstr = int_to_roman(intvalue);
}
else if (IS_EEEE(&Num))
{
@@ -6695,7 +6719,18 @@ float4_to_char(PG_FUNCTION_ARGS)
NUM_TOCHAR_prepare;
if (IS_ROMAN(&Num))
- numstr = int_to_roman((int) rint(value));
+ {
+ int32 intvalue;
+
+ /* See notes in ftoi4() */
+ value = rint(value);
+ /* On overflow, just use PG_INT32_MAX; int_to_roman will cope */
+ if (!isnan(value) && FLOAT4_FITS_IN_INT32(value))
+ intvalue = (int32) value;
+ else
+ intvalue = PG_INT32_MAX;
+ numstr = int_to_roman(intvalue);
+ }
else if (IS_EEEE(&Num))
{
if (isnan(value) || isinf(value))
@@ -6797,7 +6832,18 @@ float8_to_char(PG_FUNCTION_ARGS)
NUM_TOCHAR_prepare;
if (IS_ROMAN(&Num))
- numstr = int_to_roman((int) rint(value));
+ {
+ int32 intvalue;
+
+ /* See notes in dtoi4() */
+ value = rint(value);
+ /* On overflow, just use PG_INT32_MAX; int_to_roman will cope */
+ if (!isnan(value) && FLOAT8_FITS_IN_INT32(value))
+ intvalue = (int32) value;
+ else
+ intvalue = PG_INT32_MAX;
+ numstr = int_to_roman(intvalue);
+ }
else if (IS_EEEE(&Num))
{
if (isnan(value) || isinf(value))
diff --git a/src/test/regress/expected/int8.out b/src/test/regress/expected/int8.out
index fddc09f630..392f3bf655 100644
--- a/src/test/regress/expected/int8.out
+++ b/src/test/regress/expected/int8.out
@@ -530,6 +530,16 @@ SELECT to_char(q2, 'MI9999999999999999') FROM INT8_TBL;
-4567890123456789
(5 rows)
+SELECT to_char(q2, '9999999999999999PL') FROM INT8_TBL;
+ to_char
+--------------------
+ 456+
+ 4567890123456789+
+ 123+
+ 4567890123456789+
+ -4567890123456789
+(5 rows)
+
SELECT to_char(q2, 'FMS9999999999999999') FROM INT8_TBL;
to_char
-------------------
@@ -650,6 +660,46 @@ SELECT to_char(q2, '999999SG9999999999') FROM INT8_TBL;
456789-0123456789
(5 rows)
+SELECT to_char(q2, 'FMRN') FROM INT8_TBL;
+ to_char
+-----------------
+ CDLVI
+ ###############
+ CXXIII
+ ###############
+ ###############
+(5 rows)
+
+SELECT to_char(1234, '9.99EEEE');
+ to_char
+-----------
+ 1.23e+03
+(1 row)
+
+SELECT to_char(1234::int8, '9.99eeee');
+ to_char
+-----------
+ 1.23e+03
+(1 row)
+
+SELECT to_char(-1234::int8, '9.99eeee');
+ to_char
+-----------
+ -1.23e+03
+(1 row)
+
+SELECT to_char(1234, '99999V99');
+ to_char
+----------
+ 123400
+(1 row)
+
+SELECT to_char(1234::int8, '99999V99');
+ to_char
+----------
+ 123400
+(1 row)
+
-- check min/max values and overflow behavior
select '-9223372036854775808'::int8;
int8
diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out
index f30ac236f5..d54282496a 100644
--- a/src/test/regress/expected/numeric.out
+++ b/src/test/regress/expected/numeric.out
@@ -1991,6 +1991,21 @@ SELECT to_char(val, '9.999EEEE') FROM num_data;
-2.493e+07
(10 rows)
+SELECT to_char(val, 'FMRN') FROM num_data;
+ to_char
+-----------------
+ ###############
+ ###############
+ ###############
+ IV
+ ###############
+ ###############
+ ###############
+ ###############
+ ###############
+ ###############
+(10 rows)
+
WITH v(val) AS
(VALUES('0'::numeric),('-4.2'),('4.2e9'),('1.2e-5'),('inf'),('-inf'),('nan'))
SELECT val,
@@ -2101,6 +2116,72 @@ SELECT to_char('12345678901'::float8, 'FM9999999999D9999900000000000000000');
##########.####
(1 row)
+SELECT to_char('100'::numeric, 'rn');
+ to_char
+-----------------
+ c
+(1 row)
+
+SELECT to_char('1234'::numeric, 'rn');
+ to_char
+-----------------
+ mccxxxiv
+(1 row)
+
+SELECT to_char('1235'::float4, 'rn');
+ to_char
+-----------------
+ mccxxxv
+(1 row)
+
+SELECT to_char('1236'::float8, 'rn');
+ to_char
+-----------------
+ mccxxxvi
+(1 row)
+
+SELECT to_char('1237'::float8, 'fmrn');
+ to_char
+-----------
+ mccxxxvii
+(1 row)
+
+SELECT to_char('100e9'::numeric, 'RN');
+ to_char
+-----------------
+ ###############
+(1 row)
+
+SELECT to_char('100e9'::float4, 'RN');
+ to_char
+-----------------
+ ###############
+(1 row)
+
+SELECT to_char('100e9'::float8, 'RN');
+ to_char
+-----------------
+ ###############
+(1 row)
+
+SELECT to_char(1234.56::numeric, '99999V99');
+ to_char
+----------
+ 123456
+(1 row)
+
+SELECT to_char(1234.56::float4, '99999V99');
+ to_char
+----------
+ 123456
+(1 row)
+
+SELECT to_char(1234.56::float8, '99999V99');
+ to_char
+----------
+ 123456
+(1 row)
+
-- Check parsing of literal text in a format string
SELECT to_char('100'::numeric, 'foo999');
to_char
@@ -2297,6 +2378,12 @@ SELECT to_number('42nd', '99th');
42
(1 row)
+SELECT to_number('123456', '99999V99');
+ to_number
+-------------------------
+ 1234.560000000000000000
+(1 row)
+
RESET lc_numeric;
--
-- Input syntax
diff --git a/src/test/regress/sql/int8.sql b/src/test/regress/sql/int8.sql
index fffb28906a..53359f06ad 100644
--- a/src/test/regress/sql/int8.sql
+++ b/src/test/regress/sql/int8.sql
@@ -110,6 +110,7 @@ SELECT to_char( (q1 * -1), '9999999999999999S'), to_char( (q2 * -1), 'S999999999
FROM INT8_TBL;
SELECT to_char(q2, 'MI9999999999999999') FROM INT8_TBL;
+SELECT to_char(q2, '9999999999999999PL') FROM INT8_TBL;
SELECT to_char(q2, 'FMS9999999999999999') FROM INT8_TBL;
SELECT to_char(q2, 'FM9999999999999999THPR') FROM INT8_TBL;
SELECT to_char(q2, 'SG9999999999999999th') FROM INT8_TBL;
@@ -122,6 +123,13 @@ SELECT to_char(q2, 'FM9999999999999999.999') FROM INT8_TBL;
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;
SELECT to_char(q2, E'99999 "text" 9999 "9999" 999 "\\"text between quote marks\\"" 9999') FROM INT8_TBL;
SELECT to_char(q2, '999999SG9999999999') FROM INT8_TBL;
+SELECT to_char(q2, 'FMRN') FROM INT8_TBL;
+
+SELECT to_char(1234, '9.99EEEE');
+SELECT to_char(1234::int8, '9.99eeee');
+SELECT to_char(-1234::int8, '9.99eeee');
+SELECT to_char(1234, '99999V99');
+SELECT to_char(1234::int8, '99999V99');
-- check min/max values and overflow behavior
diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql
index c86395209a..b508cba71d 100644
--- a/src/test/regress/sql/numeric.sql
+++ b/src/test/regress/sql/numeric.sql
@@ -996,6 +996,7 @@ SELECT to_char(val, E'99999 "text" 9999 "9999" 999 "\\"text between quote marks\
SELECT to_char(val, '999999SG9999999999') FROM num_data;
SELECT to_char(val, 'FM9999999999999999.999999999999999') FROM num_data;
SELECT to_char(val, '9.999EEEE') FROM num_data;
+SELECT to_char(val, 'FMRN') FROM num_data;
WITH v(val) AS
(VALUES('0'::numeric),('-4.2'),('4.2e9'),('1.2e-5'),('inf'),('-inf'),('nan'))
@@ -1033,6 +1034,19 @@ SELECT to_char('100'::numeric, 'FM999.');
SELECT to_char('100'::numeric, 'FM999');
SELECT to_char('12345678901'::float8, 'FM9999999999D9999900000000000000000');
+SELECT to_char('100'::numeric, 'rn');
+SELECT to_char('1234'::numeric, 'rn');
+SELECT to_char('1235'::float4, 'rn');
+SELECT to_char('1236'::float8, 'rn');
+SELECT to_char('1237'::float8, 'fmrn');
+SELECT to_char('100e9'::numeric, 'RN');
+SELECT to_char('100e9'::float4, 'RN');
+SELECT to_char('100e9'::float8, 'RN');
+
+SELECT to_char(1234.56::numeric, '99999V99');
+SELECT to_char(1234.56::float4, '99999V99');
+SELECT to_char(1234.56::float8, '99999V99');
+
-- Check parsing of literal text in a format string
SELECT to_char('100'::numeric, 'foo999');
SELECT to_char('100'::numeric, 'f\oo999');
@@ -1070,6 +1084,7 @@ SELECT to_number('$1,234.56','L99,999.99');
SELECT to_number('1234.56','L99,999.99');
SELECT to_number('1,234.56','L99,999.99');
SELECT to_number('42nd', '99th');
+SELECT to_number('123456', '99999V99');
RESET lc_numeric;
--
Hi Tom, > Meh ... cfbot points out I did the float-to-int conversions wrong. > v2 attached. I'm having difficulties applying the patch. Could you please do `git format-patch` and resend it? -- Best regards, Aleksander Alekseev
Hi,
On Wed, Sep 11, 2024 at 2:33 PM Aleksander Alekseev <aleksander@timescale.com> wrote:
I'm having difficulties applying the patch. Could you please do `git
format-patch` and resend it?
Yes, I guess there is some issue with the patch but somehow I was able to apply it.
make installcheck-world -> tested, passed
Documentation -> tested, passed
Regards,
Hunaid Sohail
Aleksander Alekseev <aleksander@timescale.com> writes:
> I'm having difficulties applying the patch. Could you please do `git
> format-patch` and resend it?
patch(1) is generally far more forgiving than 'git am'.
regards, tom lane
On Sun, Sep 08, 2024 at 05:32:16PM -0400, Tom Lane wrote: > In looking at this, I found that there's also no test coverage > for the EEEE, V, or PL format codes. Also, the possibility of > overflow while converting an input value to int in order to > pass it to int_to_roman was ignored. Attached is a patch that > adds more test coverage and cleans up the Roman-numeral code > a little bit. I stared at the patch for a while, and it looks good to me. > BTW, I also discovered that there is a little bit of support > for a "B" format code: we can parse it, but then we ignore it. > And it's not documented. Oracle defines this code as a flag > that: > > Returns blanks for the integer part of a fixed-point number > when the integer part is zero (regardless of zeros in the > format model). > > It doesn't seem super hard to implement that, but given the > complete lack of complaints about it being missing, maybe we > should just rip out the incomplete support instead? AFAICT it's been like that since it was introduced [0]. I searched the archives and couldn't find any discussion about this format code. Given that, I don't have any concerns about removing it unless it causes ERRORs for calls that currently succeed, but even then, it's probably fine. This strikes me as something that might be fun for an aspiring hacker, though. [0] https://postgr.es/c/b866d2e -- nathan
Nathan Bossart <nathandbossart@gmail.com> writes:
> On Sun, Sep 08, 2024 at 05:32:16PM -0400, Tom Lane wrote:
>> In looking at this, I found that there's also no test coverage
>> for the EEEE, V, or PL format codes. Also, the possibility of
>> overflow while converting an input value to int in order to
>> pass it to int_to_roman was ignored. Attached is a patch that
>> adds more test coverage and cleans up the Roman-numeral code
>> a little bit.
> I stared at the patch for a while, and it looks good to me.
Pushed, thanks for looking!
>> BTW, I also discovered that there is a little bit of support
>> for a "B" format code: we can parse it, but then we ignore it.
> AFAICT it's been like that since it was introduced [0]. I searched the
> archives and couldn't find any discussion about this format code. Given
> that, I don't have any concerns about removing it unless it causes ERRORs
> for calls that currently succeed, but even then, it's probably fine. This
> strikes me as something that might be fun for an aspiring hacker, though.
Yeah, I left that alone for now. I don't have much interest in
making it work, but perhaps someone else will.
regards, tom lane