Re: [PATCH] Add roman support for to_number function

From: Hunaid Sohail <hunaidpgml(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, Maciek Sakrejda <m(dot)sakrejda(at)gmail(dot)com>, Tomas Vondra <tomas(at)vondra(dot)me>
Subject: Re: [PATCH] Add roman support for to_number function
Date: 2025-01-21 08:35:47
Message-ID: CAMWA6ybDR3EUiBUTeEr=2y3g_w1BSa1O3=qOqRU5E2-63c5RhA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Mon, Jan 20, 2025 at 9:25 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Hunaid Sohail 2025-01-21 08:45:12 Re: [PATCH] Add roman support for to_number function
Previous Message vignesh C 2025-01-21 08:34:06 Re: Pgoutput not capturing the generated columns