future of PQfn()

Started by Nathan Bossart11 days ago18 messageshackers
Jump to latest
#1Nathan Bossart
nathandbossart@gmail.com

PQfn() was marked "somewhat obsolete" in commit efc3a25bb0 (2003), and was
later marked "unsafe" in commit bd48114937 (2026). I looked around for
third-party code that uses this interface but found none, and the only
internal usage is for the frontend LO interface. As part of the latter
commit, a special PQnfn() function was added for use by the fronend LO
functions, but this was not exported by libpq.

Given the above, I'd like to propose retiring PQfn() in v20. Since it's an
exported symbol, we can't just delete the code, but we could have it
unconditionally error. Assuming folks are okay with that, I'm wondering
what we should do with the relevant documentation. Should we leave a stub
with a note about its removal, or should we just wipe all mentions? I'm
currently leaning towards leaving a note, but I could see the argument
that's not even worth doing given the lack of uptake.

The other question is what to do with the frontend LO code. The simplest
thing we can do is to leave PQnfn() around as an internal function that is
only used by this interface. Alternatively, we could take our own advice
and used a prepared statement with binary transmission of params/results,
but that has two key problems: 1) potential name collisions with
user-created prepared statements and 2) breakage after DISCARD/DEALLOCATE,
which I haven't come up with a good way to deal with. Another approach we
could take is to just send the query via PQexecParams(), but a simple test
(creating and unlinking 10K LOs) showed a ~41% slowdown compared to HEAD.
So, I guess we'll need to keep PQnfn() around for now...

Thoughts?

--
nathan

#2Christoph Berg
myon@debian.org
In reply to: Nathan Bossart (#1)
Re: future of PQfn()

Re: Nathan Bossart

PQfn() was marked "somewhat obsolete" in commit efc3a25bb0 (2003), and was
later marked "unsafe" in commit bd48114937 (2026). I looked around for
third-party code that uses this interface but found none

I found some references via Debian's codesearch:

https://sources.debian.org/src/fpc/3.2.2+dfsg-50/fpcsrc/packages/postgres/src/postgres3dyn.pp?hl=141#L141

PQfn : function (conn:PPGconn; fnid:longint; result_buf:Plongint; result_len:Plongint; result_is_int:longint;args:PPQArgBlock; nargs:longint):PPGresult;cdecl;
{ Accessor functions for PGresult objects }

https://sources.debian.org/src/clisp/1:2.49.20250504.gitf662209-2/modules/postgresql/postgresql.lisp?hl=375#L375

;; "Fast path" interface --- not really recommended for application use
(def-call-out PQfn (:return-type PGresult)
(:arguments (conn PGconn) (fnid int) (result_buf (c-ptr int) :out)
(result_len (c-ptr int) :out) (result_is_int int)
(args (c-array-ptr PQArgBlock)) ; at least nargs
(nargs int)))

https://sources.debian.org/src/rust-pq-sys/0.7.5-1/src/bindings_linux_32.rs?hl=745#L745

unsafe extern "C" {
pub fn PQfn(
conn: *mut PGconn,
fnid: ::std::os::raw::c_int,
result_buf: *mut ::std::os::raw::c_int,
result_len: *mut ::std::os::raw::c_int,
result_is_int: ::std::os::raw::c_int,
args: *const PQArgBlock,
nargs: ::std::os::raw::c_int,
) -> *mut PGresult;
}

https://sources.debian.org/src/vala/0.56.19-1/vapi/libpq.vapi?hl=349#L349

[CCode (cname = "PQfn")]
public Result fn (int fnid, [CCode (array_length = false)] int[] result_buf, out int result_len, int result_is_int, ArgBlock[] args);

https://sources.debian.org/src/libdbi-drivers/0.9.0-13/drivers/pgsql/dbd_pgsql.h?hl=650#L650

#define PGSQL_CUSTOM_FUNCTIONS { \
...
"PQfn", \

All seem to be libpq wrappers.

Then there is a handful of packages with a copy of libpq-fe.h that
includes the function declaration.

... plus two other mentioning PQfn as "unimplemented".

Given the above, I'd like to propose retiring PQfn() in v20. Since it's an
exported symbol, we can't just delete the code, but we could have it
unconditionally error.

Keeping the symbol but making it return an error seems ok from the
above I think.

Christoph

#3Christoph Berg
myon@debian.org
In reply to: Christoph Berg (#2)
Re: future of PQfn()

Re: To Nathan Bossart

I found some references via Debian's codesearch:

I meant to include the URL for that:

https://codesearch.debian.net/search?q=PQfn%5Cb&literal=0

PQnfn has no hits outside of postgresql-18.

Christoph

#4Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Nathan Bossart (#1)
Re: future of PQfn()

On Tue, May 26, 2026 at 9:05 AM Nathan Bossart <nathandbossart@gmail.com> wrote:

Given the above, I'd like to propose retiring PQfn() in v20. Since it's an
exported symbol, we can't just delete the code, but we could have it
unconditionally error.

That seems reasonable to me. (We may want to exercise the
PqMsg_FunctionCall message more explicitly in our test suite at the
same time we do that...)

Assuming folks are okay with that, I'm wondering
what we should do with the relevant documentation. Should we leave a stub
with a note about its removal, or should we just wipe all mentions? I'm
currently leaning towards leaving a note, but I could see the argument
that's not even worth doing given the lack of uptake.

I think we can probably wipe it out, personally.

The other question is what to do with the frontend LO code. The simplest
thing we can do is to leave PQnfn() around as an internal function that is
only used by this interface. Alternatively, we could take our own advice
and used a prepared statement with binary transmission of params/results,
but that has two key problems: 1) potential name collisions with
user-created prepared statements and 2) breakage after DISCARD/DEALLOCATE,
which I haven't come up with a good way to deal with. Another approach we
could take is to just send the query via PQexecParams(), but a simple test
(creating and unlinking 10K LOs) showed a ~41% slowdown compared to HEAD.
So, I guess we'll need to keep PQnfn() around for now...

Short-term, keeping it around seems fine.

Long-term, it doesn't feel great that the alternatives we tell other
people to use are... worse. Surely other clients of libpq run into the
layering violation problem with prepared statements, as well?

--Jacob

#5Nathan Bossart
nathandbossart@gmail.com
In reply to: Jacob Champion (#4)
Re: future of PQfn()

On Tue, May 26, 2026 at 10:42:47AM -0700, Jacob Champion wrote:

Short-term, keeping it around seems fine.

Long-term, it doesn't feel great that the alternatives we tell other
people to use are... worse. Surely other clients of libpq run into the
layering violation problem with prepared statements, as well?

Yup. Here's a related note in JDBC:

https://github.com/pgjdbc/pgjdbc/blob/cf2d89ec/docs/content/documentation/server-prepare.md?plain=1#L305-L315

I wonder how difficult it would be to teach the protocol to advise clients
when prepared statements are deallocated...

FWIW I'm less concerned about the name collision problem. I was thinking
we could just document that libpq manages statements with a prefix like
"libpq_internal_". Any problems in that area seem likely to be intentional
breakage that we needn't worry about.

--
nathan

#6Jelte Fennema-Nio
postgres@jeltef.nl
In reply to: Nathan Bossart (#1)
Re: future of PQfn()

On Tue, 26 May 2026 at 18:05, Nathan Bossart <nathandbossart@gmail.com>
wrote:

Another approach we
could take is to just send the query via PQexecParams(), but a simple test
(creating and unlinking 10K LOs) showed a ~41% slowdown compared to HEAD.
So, I guess we'll need to keep PQnfn() around for now...

Thoughts?

I had a small WIP patch (fully AI generated and not yet vetted by me)
lying around for unrelated reasons to make the extended protocol perform
closer to the simple protocol with pgbench --select-only by using
CreateOneShotCachedPlan to create the plan for unnamed prepared
statements (see attached).

Could you share the simple LO test you were running here and/or rerun it
with this patch applied? I'd love to know if the patch reduces the
slowdown significantly, or if something else is the bottleneck.

Attachments:

v1-0001-Speed-up-extended-protocol-by-using-oneshot-cache.patchtext/x-patch; charset=utf-8; name=v1-0001-Speed-up-extended-protocol-by-using-oneshot-cache.patchDownload+50-8
#7Nathan Bossart
nathandbossart@gmail.com
In reply to: Jelte Fennema-Nio (#6)
Re: future of PQfn()

On Tue, May 26, 2026 at 11:36:49PM +0200, Jelte Fennema-Nio wrote:

Could you share the simple LO test you were running here and/or rerun it
with this patch applied? I'd love to know if the patch reduces the
slowdown significantly, or if something else is the bottleneck.

The test is just:

int main()
{
PCconn *conn = PQsetdb(NULL, NULL, NULL, NULL, "postgres");

for (int i = 0; i < 1000000; i++)
lo_create(conn, i);

for (int i = 0; i < 1000000; i++)
lo_unlink(conn, i);
}

Before applying your patch -> 0.457 seconds (best of ~10)
After applying your patch -> 0.444 seconds (best of ~10)

For reference, HEAD runs this test in 0.319 seconds.

I've attached my work-in-progress patches here, in case you're interested.
0001 switches the frontend LO interface to use prepared statements, and
0002 removes PQfn() entirely. 0003 applies on top of those two and
switches the frontend LO interface to use PQexecParams() instead.

--
nathan

Attachments:

v1-0001-stop-using-PQfn-in-libpq-s-LO-interface.patchtext/plain; charset=us-asciiDownload+299-383
v1-0002-remove-support-for-PQfn.patchtext/plain; charset=us-asciiDownload+12-416
v1-0003-LO-frontend-PQexecParams.patchtext/plain; charset=us-asciiDownload+26-213
#8Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Nathan Bossart (#5)
Re: future of PQfn()

On Tue, May 26, 2026 at 12:55 PM Nathan Bossart
<nathandbossart@gmail.com> wrote:

I wonder how difficult it would be to teach the protocol to advise clients
when prepared statements are deallocated...

Probably not too difficult. But it seems like most, if not all, of the
stuff in the DISCARD ALL umbrella is a target for a feature like
that... This feels a lot like the perennial request for proxies to be
able to separate their own context from the per-application/per-user
contexts running on top of them.

--Jacob

#9Nathan Bossart
nathandbossart@gmail.com
In reply to: Jacob Champion (#8)
Re: future of PQfn()

On Wed, May 27, 2026 at 02:39:53PM -0700, Jacob Champion wrote:

On Tue, May 26, 2026 at 12:55 PM Nathan Bossart
<nathandbossart@gmail.com> wrote:

I wonder how difficult it would be to teach the protocol to advise clients
when prepared statements are deallocated...

Probably not too difficult. But it seems like most, if not all, of the
stuff in the DISCARD ALL umbrella is a target for a feature like
that... This feels a lot like the perennial request for proxies to be
able to separate their own context from the per-application/per-user
contexts running on top of them.

I've been thinking through a bunch of options here, and a new protocol
message seems like the best choice, if for no other reason than it solves a
general problem for clients. I'm working on a patch that I'll post in a
new thread.

--
nathan

#10Nathan Bossart
nathandbossart@gmail.com
In reply to: Jacob Champion (#8)
Re: future of PQfn()

On Wed, May 27, 2026 at 02:39:53PM -0700, Jacob Champion wrote:

On Tue, May 26, 2026 at 12:55 PM Nathan Bossart
<nathandbossart@gmail.com> wrote:

I wonder how difficult it would be to teach the protocol to advise clients
when prepared statements are deallocated...

Probably not too difficult. But it seems like most, if not all, of the
stuff in the DISCARD ALL umbrella is a target for a feature like
that... This feels a lot like the perennial request for proxies to be
able to separate their own context from the per-application/per-user
contexts running on top of them.

Here is a work-in-progress patch set that goes this direction. This
introduces a callback mechanism in libpq that is used to handle statement
deallocation notifications. Older servers/clients fall back to
PQexecParams(), which is slower, but the alternative is to leave PQnfn()
and related code around indefinitely.

I'm wondering whether this new message type is general enough. For
example, perhaps we could make an extensible message type for tracking
various things. And I want to ensure this is useful for other clients,
too.

--
nathan

Attachments:

v2-0001-tell-client-when-prep-stmts-are-deallocated.patchtext/plain; charset=us-asciiDownload+131-8
v2-0002-stop-using-PQfn-in-libpq-s-LO-interface.patchtext/plain; charset=us-asciiDownload+319-384
v2-0003-remove-support-for-PQfn.patchtext/plain; charset=us-asciiDownload+12-420
#11Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Nathan Bossart (#10)
Re: future of PQfn()

On Fri, May 29, 2026 at 8:14 AM Nathan Bossart <nathandbossart@gmail.com> wrote:

Here is a work-in-progress patch set that goes this direction.

At a high level, I think advertising support for a single new message
needs to be done in a protocol extension rather than a minor version
bump.

This
introduces a callback mechanism in libpq that is used to handle statement
deallocation notifications. Older servers/clients fall back to
PQexecParams(), which is slower, but the alternative is to leave PQnfn()
and related code around indefinitely.

IMO there's no hurry in getting rid of that path. If we decide to go
this direction, a fallback to PQnfn() seems like it'd fine for a few
releases; we could eventually swap to a PQexecParams() fallback and
get rid of the extra code once the older servers have aged out.

I'm wondering whether this new message type is general enough. For
example, perhaps we could make an extensible message type for tracking
various things. And I want to ensure this is useful for other clients,
too.

If it's just a general notification message, what does negotiating
"support" mean? Is best-effort notification okay, if the client has no
idea what a future message type means, or if the server doesn't send
the specific type of message the client is hoping for?

(In general, I'm kind of down on the "notify the client that X
happened" method of working around architectural issues. Maybe that's
what we need to move this specific part forward, but it doesn't feel
like a long-term solution and I don't know that we need to genericize
it without a solid set of use cases.)

--Jacob

#12Nathan Bossart
nathandbossart@gmail.com
In reply to: Jacob Champion (#11)
Re: future of PQfn()

On Fri, May 29, 2026 at 08:43:07AM -0700, Jacob Champion wrote:

On Fri, May 29, 2026 at 8:14 AM Nathan Bossart <nathandbossart@gmail.com> wrote:

Here is a work-in-progress patch set that goes this direction.

At a high level, I think advertising support for a single new message
needs to be done in a protocol extension rather than a minor version
bump.

WFM

This
introduces a callback mechanism in libpq that is used to handle statement
deallocation notifications. Older servers/clients fall back to
PQexecParams(), which is slower, but the alternative is to leave PQnfn()
and related code around indefinitely.

IMO there's no hurry in getting rid of that path. If we decide to go
this direction, a fallback to PQnfn() seems like it'd fine for a few
releases; we could eventually swap to a PQexecParams() fallback and
get rid of the extra code once the older servers have aged out.

That's fine with me, too.

I'm wondering whether this new message type is general enough. For
example, perhaps we could make an extensible message type for tracking
various things. And I want to ensure this is useful for other clients,
too.

If it's just a general notification message, what does negotiating
"support" mean? Is best-effort notification okay, if the client has no
idea what a future message type means, or if the server doesn't send
the specific type of message the client is hoping for?

That's what I had in mind. But if we don't have anything specific in mind
that this mechanism could be extended to support, maybe we shouldn't
bother. Especially if we can just add protocol extensions as necessary.

(In general, I'm kind of down on the "notify the client that X
happened" method of working around architectural issues. Maybe that's
what we need to move this specific part forward, but it doesn't feel
like a long-term solution and I don't know that we need to genericize
it without a solid set of use cases.)

I'm certainly open to other ideas, but I'm afraid this is the best I've
come up with in my admittedly limited time thinking about the problem.

--
nathan

#13Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Nathan Bossart (#12)
Re: future of PQfn()

On Fri, May 29, 2026 at 9:11 AM Nathan Bossart <nathandbossart@gmail.com> wrote:

I'm certainly open to other ideas, but I'm afraid this is the best I've
come up with in my admittedly limited time thinking about the problem.

No worries -- I hadn't meant to block progress here on protocol
design. I think keeping PQnfn() for the immediate future is a good
plan. I just wanted to plant a seed for getting away from this problem
eventually.

(As for pie-in-the-sky alternative ideas, the ability for middleware
to separate contexts or streams of packets has come up before. libpq
could theoretically mark its own "context" of server-side allocations
that are not touched by an application-context DISCARD.)

--Jacob

#14Nathan Bossart
nathandbossart@gmail.com
In reply to: Jacob Champion (#13)
Re: future of PQfn()

For future reference in the archives, I'm moving the discussion about 0001
(the prepared statement deallocation notification mechanism) to a new
thread:

/messages/by-id/ahm_4eOKkkKJ3Gds@nathan

On Fri, May 29, 2026 at 09:33:03AM -0700, Jacob Champion wrote:

On Fri, May 29, 2026 at 9:11 AM Nathan Bossart <nathandbossart@gmail.com> wrote:

I'm certainly open to other ideas, but I'm afraid this is the best I've
come up with in my admittedly limited time thinking about the problem.

No worries -- I hadn't meant to block progress here on protocol
design. I think keeping PQnfn() for the immediate future is a good
plan. I just wanted to plant a seed for getting away from this problem
eventually.

(As for pie-in-the-sky alternative ideas, the ability for middleware
to separate contexts or streams of packets has come up before. libpq
could theoretically mark its own "context" of server-side allocations
that are not touched by an application-context DISCARD.)

Along these lines, I did consider "pinning" statements or even having
"built-in" ones for libpq. I didn't like the "pinning" idea because that
seemed problematic for connection poolers. And the "built-in" idea seemed
too libpq-centric for what I'd argue is a general problem. The other ideas
involved guessing at what's happening based on the queries or somehow
trying to handle failures due to missing/wrong prepared statements, none of
which felt viable.

--
nathan

#15Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Nathan Bossart (#14)
Re: future of PQfn()

On Fri, May 29, 2026 at 9:42 AM Nathan Bossart <nathandbossart@gmail.com> wrote:

Along these lines, I did consider "pinning" statements or even having
"built-in" ones for libpq. I didn't like the "pinning" idea because that
seemed problematic for connection poolers.

Right -- whether a general context, or multiplexed streams, or
explicit pins, proxies would have to be intimately aware of them. They
can then layer their own on top, or make sure different client
requests don't conflict, or release them on client disconnection...

And the "built-in" idea seemed
too libpq-centric for what I'd argue is a general problem. The other ideas
involved guessing at what's happening based on the queries or somehow
trying to handle failures due to missing/wrong prepared statements, none of
which felt viable.

Agreed. (Although, for that last point, I wondered whether we could
make this idempotent somehow. I think the answer is "not worth it".)

--Jacob

#16Nathan Bossart
nathandbossart@gmail.com
In reply to: Jacob Champion (#15)
Re: future of PQfn()

While the prepared statement deallocation idea bakes in the other thread, I
thought I'd post a new patch set here. I've reordered things a bit and
left out the conversion to prepared statements for now (although I do have
a patch for that). 0001 removes PQfn() and changes all internal uses to
PQnfn(). 0002 and 0003 are just some additional cleanup that I noticed
as I've been working on this stuff.

--
nathan

Attachments:

v3-0001-remove-PQfn.patchtext/plain; charset=us-asciiDownload+60-146
v3-0002-remove-lo_hton64-and-lo_ntoh64.patchtext/plain; charset=us-asciiDownload+4-55
v3-0003-add-helper-functions-for-fast-path-arg-setup.patchtext/plain; charset=us-asciiDownload+38-67
In reply to: Nathan Bossart (#16)
Re: future of PQfn()

Nathan Bossart <nathandbossart@gmail.com> writes:

- <sect1 id="libpq-fastpath">
- <title>The Fast-Path Interface</title>
-
- <indexterm zone="libpq-fastpath">
- <primary>fast path</primary>
- </indexterm>

Should we move this to the "Obsolete or Renamed Features" appendix
(e.g. appendix-obsolete-libpq-fastpath.sgml) with a description of why
it was removed?

- ilmari

#18Nathan Bossart
nathandbossart@gmail.com
In reply to: Dagfinn Ilmari Mannsåker (#17)
Re: future of PQfn()

On Thu, Jun 04, 2026 at 11:39:02AM +0100, Dagfinn Ilmari Mannsåker wrote:

Nathan Bossart <nathandbossart@gmail.com> writes:

- <sect1 id="libpq-fastpath">
- <title>The Fast-Path Interface</title>
-
- <indexterm zone="libpq-fastpath">
- <primary>fast path</primary>
- </indexterm>

Should we move this to the "Obsolete or Renamed Features" appendix
(e.g. appendix-obsolete-libpq-fastpath.sgml) with a description of why
it was removed?

Yes, I think we should. Done in the attached.

--
nathan

Attachments:

v4-0001-remove-PQfn.patchtext/plain; charset=us-asciiDownload+86-146
v4-0002-remove-lo_hton64-and-lo_ntoh64.patchtext/plain; charset=us-asciiDownload+4-55
v4-0003-add-helper-functions-for-fast-path-arg-setup.patchtext/plain; charset=us-asciiDownload+38-73