pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
Cc: Jacob Champion <pchampion@vmware.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Cc: Daniel Gustafsson <daniel@yesql.se>
Subject: Re: [PATCH] Make jsonapi usable from libpq
Date: Mon, 28 Jun 2021 15:15:47 -0400
Message-ID: <335902.1624907747@sss.pgh.pa.us> (raw)
In-Reply-To: <3131385.1624746109@sss.pgh.pa.us>
References: <a250d475ba1c0cc0efb7dfec8e538fcc77cdcb8e.camel@vmware.com>
	<YNQedHwheUaTnnGt@paquier.xyz>
	<daeb22ec6ca8ef61e94d766a9b35fb03cabed38e.camel@vmware.com>
	<YNZ2mnsbDVJQrA/a@paquier.xyz>
	<3045186.1624729430@sss.pgh.pa.us>
	<3122443.1624735363@sss.pgh.pa.us>
	<3131385.1624746109@sss.pgh.pa.us>

I wrote:
>> Not real sure what to do about PGTHREAD_ERROR.

> I wonder if we shouldn't simply nuke that macro and change the
> call sites into "Assert(false)".

After further study this still seems like the best available choice.
We do not have the option to make either default_threadlock() or
pq_lockingcallback() do something saner, like return a failure
indication.  pq_lockingcallback()'s API is dictated by OpenSSL,
while default_threadlock()'s API is exposed to users by libpq
(IOW, we could have gotten that one right years ago, but we
failed to, and it seems much too late to change it now).

Also, I trawled the mailing list archives, and I can find no
indication that any of the PGTHREAD_ERROR messages have ever
been seen in the field.  The last relevant discussion seems
to be in

https://www.postgresql.org/message-id/flat/20130801142443.GO2706%40tamriel.snowman.net

where it was observed that this code isn't very well thought
through :-(

My proposal is to replace PGTHREAD_ERROR by Assert(false)
in HEAD, but leave things alone in the back branches.

As far as the other patch to check for mistakes with "nm"
goes, we could either do nothing in the back branches, or
install a check for "exit" only, not "abort".  But there's
probably no real need for such a check in the back branches
as long as we're enforcing it in HEAD.

			regards, tom lane





view thread (21+ messages)  latest in thread

Message-ID: <335902.1624907747@sss.pgh.pa.us>
Permalink:  ../335902.1624907747@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/335902.1624907747@sss.pgh.pa.us

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, michael@paquier.xyz, pchampion@vmware.com, daniel@yesql.se
  Subject: Re: [PATCH] Make jsonapi usable from libpq
  In-Reply-To: <335902.1624907747@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox