Re: [PATCH] Make jsonapi usable from libpq

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

In response to

Responses

Browse pgsql-hackers by date

  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