On Thu, Dec 07, 2023 at 10:28:50PM -0500, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>> I did both of these in v2, although I opted to test that the first
>> character after the optional '-' was a digit instead of testing that it was
>> _not_ an 'I' or 'N'.
>
> Yeah, I thought about that too after sending my message. This version
> LGTM, although maybe the comment could be slightly more verbose with
> explicit reference to Inf/NaN as being the cases we need to quote.
Done.
>> I think there are some similar improvements that we can make for
>> JSONTYPE_BOOL and JSONTYPE_CAST, but I haven't tested them yet.
>
> I am suspicious of using
>
> appendStringInfo(result, "\"%s\"", ...);
>
> in each of these paths; snprintf is not a terribly cheap thing.
> It might be worth expanding that to appendStringInfoChar/
> appendStringInfoString/appendStringInfoChar.
WFM. I'll tackle JSONTYPE_BOOL and JSONTYPE_CAST next...
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
