Thanks for the contribution. I took a look at the patch, and it works as advertised. It's too late for the September commitfest, but I took the liberty of registering your patch for the November CF [1]. In the course of that, I found an older thread proposing this feature seven years ago [2]. That patch was returned with feedback and (as far as I can tell), was not followed-up on by the author. You may want to review that thread for feedback; I won't repeat it here.
On Fri, Aug 30, 2024 at 12:22 AM Hunaid Sohail <hunaidp...@gmail.com> wrote: >While looking at formatting.c file, I noticed a TODO about "add support for >roman number to standard number conversion" >(https://github.com/postgres/postgres/blob/master/src/backend/utils/adt/formatting.c#L52) Your patch should also remove the TODO =) > - Is it okay for the function to handle Roman numerals in a case-insensitive > way? (e.g., 'XIV', 'xiv', and 'Xiv' are all seen as 14). The patch in the thread I linked also took a case-insensitive approach. I did not see any objections to that, and it seems reasonable to me as well. > - How should we handle Roman numerals with leading or trailing spaces, like ' > XIV' or 'MC '? Should we trim the spaces, or would it be better to throw an > error in such cases? I thought we could reference existing to_number behavior here, but after playing with it a bit, I'm not really sure what that is: -- single leading space maciek=# select to_number(' 1', '9'); to_number ----------- 1 (1 row) -- two leading spaces maciek=# select to_number(' 1', '9'); ERROR: invalid input syntax for type numeric: " " -- two leading spaces and template pattern with a decimal maciek=# select to_number(' 1', '9D9'); to_number ----------- 1 (1 row) Separately, I also noticed some unusual Roman representations work with your patch: postgres=# select to_number('viv', 'RN'); to_number ----------- 9 (1 row) Is this expected? In contrast, some somewhat common alternative representations don't work: postgres=# select to_number('iiii', 'RN'); ERROR: invalid roman numeral I know this is expected, but is this the behavior we want? If so, we probably want to reject the former case, too. If not, maybe that one is okay, too. I know I've probably offered more questions than answers, but I hope finding the old thread here is useful. Thanks, Maciek [1]: https://commitfest.postgresql.org/50/5233/ [2]: https://www.postgresql.org/message-id/flat/CAGMVOduAJ9wKqJXBYnmFmEetKxapJxrG3afUwpbOZ6n_dWaUnA%40mail.gmail.com
