From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Jacob Champion <pchampion(at)vmware(dot)com> |
Cc: | "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PATCH] Make jsonapi usable from libpq |
Date: | 2021-06-24 05:56:04 |
Message-ID: | YNQedHwheUaTnnGt@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Jun 22, 2021 at 10:59:37PM +0000, Jacob Champion wrote:
> Hmm. I'm honestly hesitant to start splitting files apart, mostly
> because json_log_and_abort() is only called from two places, and both
> those places are triggered by programmer error as opposed to user
> error.
>
> Would it make more sense to switch to an fprintf-and-abort case, to
> match the approach taken by PGTHREAD_ERROR and the out-of-memory
> conditions in fe-print.c? Or is there already precedent for handling
> can't-happen code paths with in-band errors, through the PGconn?
Not really..
Looking more closely at that, I actually find a bit crazy the
requirement for any logging within jsonapi.c just to cope with the
fact that json_errdetail() and report_parse_error() just want to track
down if the caller is giving some garbage or not, which should never
be the case, really. So I would be tempted to eliminate this
dependency to begin with.
The second thing is how we should try to handle the way the error
message gets allocated in json_errdetail(). libpq cannot rely on
psprintf(), so I can think about two options here:
- Let the caller of json_errdetail() allocate the memory area of the
error message by itself, pass it down to the function.
- Do the allocation within json_errdetail(), and let callers cope the
case where json_errdetail() itself fails on OOM for any frontend code
using it.
Looking at HEAD, the OAUTH patch and the token handling, the second
option looks more interesting. I have to admit that handling the
token part makes the patch a bit special, but that avoids duplicating
all those error messages for libpq. Please see the idea as attached.
--
Michael
Attachment | Content-Type | Size |
---|---|---|
jsonapi-libpq.patch | text/x-diff | 9.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2021-06-24 06:34:26 | Re: Can a child process detect postmaster death when in pg_usleep? |
Previous Message | Michael Paquier | 2021-06-24 05:33:59 | Re: Decoding speculative insert with toast leaks memory |