Re: [PATCH] Add roman support for to_number function

Hi,

On Mon, Jan 20, 2025 at 9:25 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> I've not tracked down the exact cause of that, but I think it
> has to do with the fact that NUM_numpart_from_char() is willing
> to eat a single leading space per call, even if it's not the
> first call.  The original author's reasoning for that is lost
> to history thanks to the lack of comments, but I believe that
> the idea was not "eat random whitespace" but "consume a space
> that was emitted as a substitute for a plus sign".  The fact
> that it can happen once per '9' code feels more like a bug than
> something intentional.  I'm not proposing that we do something
> about that, at least not today, but it doesn't seem like
> something we want to model RN's behavior on either.  So I'm
> not in favor of allowing embedded spaces.
>

Agreed. We should not allow trailing and embedded spaces.
Since, trailing spaces and become embedded spaces like
'MCC  ' or 'M CC'.
It means cases like these should be invalid:
SELECT to_number('MMXX  ', 'RN');
SELECT to_number('  XIV  ', '  RN');
SELECT to_number('M CC', 'RN');


> So on the argument that to_number's RN should be willing to
> consume what to_char's RN produces, we're both wrong and
> roman_to_int should be willing to eat leading spaces.
> What do you think?
>

Agreed. Your changes in v7 consumes leading spaces if leading space
is present in format ('  RN'). But we need it to work on cases like:
SELECT to_number('  XIV', 'RN');
So, I can add this logic in my next patch.

Regards,
Hunaid Sohail