Re: Support json_errdetail in FRONTEND builds

On 2024-03-12 Tu 14:43, Jacob Champion wrote:
> Hello,
>
> Both the incremental JSON [1] and OAuth [2] patchsets would be
> improved by json_errdetail(), which was removed from FRONTEND builds
> in b44669b2ca:
>
>> The routine providing a detailed error message based on the error code
>> is made backend-only, the existing code being unsafe to use in the
>> frontend as the error message may finish by being palloc'd or point to a
>> static string, so there is no way to know if the memory of the message
>> should be pfree'd or not.
> Attached is a patch to undo this, by attaching any necessary
> allocations to the JsonLexContext so the caller doesn't have to keep
> track of them.
>
> This is based on the first patch of the OAuth patchset, which
> additionally needs json_errdetail() to be safe to use from libpq
> itself. Alvaro pointed out offlist that we don't have to go that far
> to re-enable this function for the utilities, so this patch is a sort
> of middle ground between what we have now and what OAuth implements.
> (There is some additional minimization that could be done to this
> patch, but I'm hoping to keep the code structure consistent between
> the two, if the result is acceptable.)

Seems reasonable.

>
> Two notes that I wanted to point out explicitly:
> - On the other thread, Daniel contributed a destroyStringInfo()
> counterpart for makeStringInfo(), which is optional but seemed useful
> to include here.

yeah, although maybe worth a different patch.

> - After this patch, if a frontend client calls json_errdetail()
> without calling freeJsonLexContext(), it will leak memory.

Not too concerned about this:

1. we tend to be a bit more relaxed about that in frontend programs,
especially those not expected to run for long times and especially on
error paths, where in many cases the program will just exit anyway.

2. the fix is simple where it's needed.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com