Re: [PATCH] Add roman support for to_number function - Mailing list pgsql-hackers

Thank you for the tweaks to the patch. Overall, it looks great.

Initially, when I looked at the test case:

SELECT to_number('M CC', 'RN');

I was confused about whether it should be 'MCC'. After further inspection,

I realized that the output is 1000 for 'M'. The format of the input can be a

bit unclear. Perhaps we could add a note in the documentation

or issue a warning in roman_to_int function when input is truncated,

to clarify this behavior.

+       bool            truncated = false;
+
+       /*
+        * Collect and decode valid roman numerals, consuming at most
+        * MAX_ROMAN_LEN characters.  We do this in a separate loop to avoid
+        * repeated decoding and because the main loop needs to know when it's at
+        * the last numeral.
+        */
+       for (len = 0; len < MAX_ROMAN_LEN && !OVERLOAD_TEST; len++)
+       {
+               char            currChar = pg_ascii_toupper(*Np->inout_p);
+               int                     currValue = ROMAN_VAL(currChar);
+
+               if (currValue == 0)
+               {
+                       truncated = true;
+                       break;                          /* Not a valid roman numeral. */
+               }
+               romanChars[len] = currChar;
+               romanValues[len] = currValue;
+               Np->inout_p++;
+       }
+
+       if (len == 0)
+               return -1;                              /* No valid roman numerals. */
+
+       /* Check for truncation. */
+       if (truncated)
+       {
+               ereport(WARNING,
+                               (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+                                errmsg("Input truncated to \"%.*s\"",
+                                               len, romanChars)));
+       }