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

From Hunaid Sohail
Subject Re: [PATCH] Add roman support for to_number function
Date
Msg-id CAMWA6ybDR3EUiBUTeEr=2y3g_w1BSa1O3=qOqRU5E2-63c5RhA@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] Add roman support for to_number function  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [PATCH] Add roman support for to_number function
List pgsql-hackers
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 

pgsql-hackers by date:

Previous
From: vignesh C
Date:
Subject: Re: Pgoutput not capturing the generated columns
Next
From: Hunaid Sohail
Date:
Subject: Re: [PATCH] Add roman support for to_number function