public inbox for [email protected]
help / color / mirror / Atom feedFrom: Zsolt Parragi <[email protected]>
To: Jacob Champion <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Chao Li <[email protected]>
Subject: Re: [oauth] Stabilize the libpq-oauth ABI (and allow alternative implementations?)
Date: Fri, 6 Mar 2026 22:44:43 +0000
Message-ID: <CAN4CZFOMv=0Y1PZ8uw3xgJmEqt4D1dr1-yOq9jjZYTARZp7H9Q@mail.gmail.com> (raw)
In-Reply-To: <CAOYmi+nRmt+K+ZVNztKFv=LuasDCCSsXLihZB6BEkVeroW8EvA@mail.gmail.com>
References: <CAOYmi+mrGg+n_X2MOLgeWcj3v_M00gR8uz_D7mM8z=dX1JYVbg@mail.gmail.com>
<[email protected]>
<CAOYmi+mwzS4xfgomL1Lte+W1hRk+zahhZLnRMDJ8nYxoHUWt_w@mail.gmail.com>
<CAOYmi+mEU_q9sr1PMmE-4rLwFN=OjyndDwFZvpsMU3RNJLrM9g@mail.gmail.com>
<CAN4CZFPwdH_GMrappThju=t4m5n95Dse6ecTNwN=h4GpZLOMng@mail.gmail.com>
<CAOYmi+m-hphbx=CRfGSdcrHi-jL3OrZGJMTAPOYp=OK253hBTA@mail.gmail.com>
<CAOYmi+nnYr_Oe=KH2ZW6Ld88eZWv5P-s5ZtWOEh2KsPtMqOzbg@mail.gmail.com>
<CAOYmi+nCg5upBVOo_UCSjMfO=YMkZXcSEsgaADKXqerr5wahZQ@mail.gmail.com>
<CAOYmi+nRmt+K+ZVNztKFv=LuasDCCSsXLihZB6BEkVeroW8EvA@mail.gmail.com>
For the first commits I only have a few more questions/comments about
the error messages, otherwise looks good.
> > + libpq_append_conn_error(conn, "failed to lock mutex (%d)", lockerr);
> > + return 0;
> > + }
> > Shouldn't this path return -1,
>
> We could. I chose zero to try to retain the PG18 behavior, but I could
> expand this error message and set request->error instead. If that'd be
> less confusing to you as a reader, it's probably worth the change.
If this returns 0, we print out
failed to lock mutex
no OAuth flows are available (try installing the libpq-oauth package)
Which isn't ideal, as the library is there, so installing the package
wouldn't help.
+ if ((start_flow = dlsym(state->flow_module, "pg_start_oauthbearer")) == NULL)
And this path has the same issue, the library is there, so suggesting
to install libpq-oauth isn't helpful.
The more detailed message is only printed out with unsafe debugging,
without that it just returns 0.
+ appendPQExpBuffer(&conn->errorMessage,
+ "use_builtin_flow: failed to lock mutex (%d)\n",
+ lockerr);
This is after an assert, so maybe it is okay as is, but this bypasses
gettext. (or shouldn't it use "internal error:" similarly to the other
untranslated error message? and another 2 internal errors are
translated)
> (try installing the libpq-oauth package)
This isn't changed in these patches, but Is it okay to assume a
package name here? This is not a package that universally exists
everywhere, we can't even be sure that pg was installed with a package
manager. On RHEL it is called postgresql18-libs-oauth, on suse it's
part of the main libpq package. In both cases if for some internal
error it can't find/load the library, we suggest installing a package
that doesn't exist on that system.
view thread (19+ messages) latest in thread
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: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: [oauth] Stabilize the libpq-oauth ABI (and allow alternative implementations?)
In-Reply-To: <CAN4CZFOMv=0Y1PZ8uw3xgJmEqt4D1dr1-yOq9jjZYTARZp7H9Q@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox