Re: [PATCH] Add roman support for to_number function

Hunaid Sohail <hunaidpgml(at)gmail(dot)com> writes:
> I’ve attached a new patch that addresses comments 2, 3, and 4 from Tomas.

I'm still quite unhappy that this doesn't tolerate trailing
whitespace. That's not how other format patterns work, and it makes
it impossible to round-trip the output of to_char(..., 'RN').
I think the core of the problem is that you tried to cram the logic
in where the existing "not implemented" error is thrown. But on
inspection there is nothing sane about that entire "Roman correction"
stanza -- what it does is either useless (zeroing already-zeroed
fields) or in the wrong place (if we don't want to allow other flags
with IS_ROMAN, that should be done via an error in NUMDesc_prepare,
like other similar cases). In the attached I moved the roman_to_int
call into NUM_processor's main loop so it's more like other format
patterns, and fixed it to not eat any more characters than it should.
This might allow putting back some format-combination capabilities,
but other than the whitespace angle I figure we can leave that for
people to request it.

regards, tom lane