On Tue, Dec 16, 2025 at 9:40 AM Jacob Champion
<jacob.champion@enterprisedb.com> wrote:
> Open questions remain:
> 1) 0004: Any objections to putting PQExpBuffer into libpq-fe.h?
Tom sounded lukewarm to this on the Discord, so I looked into
replacing the new usage with a simple char*.
That actually exposed a bug: appending error data during cleanup
doesn't help us at all, because we've already stopped using the error
buffer. In fact my whole idea of adding things to conn->errorMessage
during a teardown operation has been nearly useless from the
beginning. Elsewhere in libpq, we handle cases like these (which
should ideally just not happen) by printing warnings to stderr, so
v3-0001 does that instead.
Besides exposing a bug, switching to char* means anyone who's not
programming in C and already has code that handles pointer ownership
across the boundary can continue to make use of that, instead of
adapting to a completely new API for this one particular use case. We
already have the cleanup-callback requirement for the token anyway;
it's fine.
> 2) 0004: Thoughts on the v2 inheritance struct style as opposed to
> relying on implementations to double-check the struct length?
Still feels half-dozen or the other to me, so I'm planning to move
ahead with the inheritance model. I'll look into
VALGRIND_MAKE_MEM_NOACCESS (and/or other ways to catch people
scribbling where they shouldn't) for v4.
> 3) 0005: Should I add the thread lock to an init() API, or expose a
> new PQgetThreadLock() that other code can use?
v3-0002 implements PQgetThreadLock(). The conversation with Nico
Williams at [1] cemented this for me; it's entirely possible that
implementations will need the lock at other times besides
initialization, say if they're mixing OAuth with Kerberos. Adding a
way to retrieve it doesn't actually expose new functionality --
applications could always get at our internal implementation by
calling PQregisterThreadLock(NULL) -- but libraries can't use that API
safely.
Also, clients can probably make use of some of the newer ways of doing
this kind of initialization (pthread_once, etc.) that weren't in wide
use back when the init-function design showed up. We may not ever
actually need a separate init function, and if I'm wrong we can always
add one.
I think I'll put this in its own top-level thread for comment.
> (In other words, a plugin architecture causes the compile-time
> and run-time dependency arrows to point in opposite directions, so
> plugins won't be able to rely on the LIBPQ_HAS_* macros to determine
> what APIs are available to them.)
>
> (TODO: Are there implications for our use of RTLD_NOW at dlopen() time?
To answer my own question, yes: any future libpq-oauth plugin that
needs PG20+ APIs will have to lazy-load them (weak symbol
declarations, etc.) or else accept that long-running applications may
fail OAuth connections until they restart.
A more general plugin system would need to solve this. For example, we
could load RTLD_LAZY, check for a minimum libpq version declared by
the plugin, and then either upgrade the bindings with RTLD_NOW, or
dlclose() and bail. But I don't think I need to be solving a
nonexistent problem now.
--Jacob
[1] https://postgr.es/m/aSSp03wmNMngi/Oe%40ubby
