Re: [PATCH] Add roman support for to_number function
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Hunaid Sohail <hunaidpgml@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org,
Maciek Sakrejda <m.sakrejda@gmail.com>,
Tomas Vondra <tomas@vondra.me>
Date: 2025-01-18T00:27:43Z
Lists: pgsql-hackers
Attachments
- v7-0001-Add-roman-support-for-to_number-function.patch (text/x-diff) patch v7-0001
Hunaid Sohail <hunaidpgml@gmail.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
Commits
-
Support RN (roman-numeral format) in to_number().
- 172e6b3adb2e 18.0 landed