Re: Test improvements and minor code fixes for formatting.c. - Mailing list pgsql-hackers

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;

 --