Using Close protocol message instead of DEALLOCATE to help prepared statements in PgBouncer

[JelteF]

PgBouncer does currently not parse queries at all, only protocol messages. But psycopg closes its statements using DEALOCATE sql call:

psycopg/psycopg/psycopg/_preparing.py

Line 119 in 6380363

self._maint_commands.append(b"DEALLOCATE " + name)

What do you think of about, instead of sending DEALLOCATE text query here, to instead send a Close protocol message? Because that would make it work automatically with the work-in-progress PgBouncer prepared statements patch: pgbouncer/pgbouncer#845

P.S. The DEALLOCATE ALL usage is similarly problematic, but maybe PgBouncer can make an exception for supporting that since it would be extremely easy to parse. And also it should not be necessary to use as I explained a few minutes ago on the PG mailing list thread that triggered this DEALLOCATE ALL usage (according to git blame): https://www.postgresql.org/message-id/flat/CAGECzQSUPTjfb7BYLg5c22tN4SSuLTRv9-6TpuZTpJ0q8hqp2w%40mail.gmail.com#b9b91ddf3d37160a3e8771e69b27f0ac

[dvarrazzo]

Prepared statements and pgbouncer are fundamentally incompatible. There is not expectation that they work. If you are using pgbouncer you must disable prepared statements.

We would to it ourselves, but PgBouncer offers no way to detect them: see pgbouncer/pgbouncer#653

I also don't see how a Close message can replace DEALLOCATE.

[JelteF]

Could you please reopen the issue? I probably wasn't clear on my friday end of day (and I updated my initial message a bit to hopefully clarify). But I'm one of the PgBouncer maintainers (although for less than a year). And I'm trying to suggest a (to me seemingly) small change in psycopg, so that it would work with the prepared statement support in transaction pooling mode that's currently actively being worked on for PgBouncer.

[JelteF]

I also don't see how a Close message can replace DEALLOCATE.

Afaict Close is the protocol level equivalent of DEALLOCATE (except it does not support ALL). From the Postgres protocol docs (emphasis mine):

The Close message closes an existing prepared statement or portal and releases resources.

Source: https://www.postgresql.org/docs/current/protocol-flow.html#PROTOCOL-FLOW-EXT-QUERY

[dvarrazzo]

Ok, I thought Close closed the connection.

Does the libpq have a function to send a close? If not, then no, we cannot. We don't talk to the database at wire protocol level, only at libpq level.

As far as I see it, it is unwise to change psycopg to accommodate pgbouncer if you are developing an experimental feature. You are free to change psycopg code and propose a change but maybe only after your code is ready.

Also please know that I can see improvements possible in the communication between psycopg and pgbouncer, but if pgbouncer even refuses to identify itself and advertise its most important settings (does it support transactions or not?) then it's a non-starter.

[dvarrazzo]

For instance, just saying, we could even avoid to run DEALLOCATE at all on pgbouncer and trust that it will do its own garbage collection and dispose of connection after some use, so that prepared queries don't accumulate forever. But we could do that only if we see that we are connected to pgbouncer, and have some sort of "capabilities" object on the connection to manage the nuances.

Without pgbouncer/pgbouncer#653 there's no place we can go. If the attitude of the project is to pretend that there is no difference between a pgbouncer connection vs. a native one, then you will have to deal with DEALLOCATE or whatever else we can legally throw at postgres, and assume unaltered semantics.

[JelteF]

After trying to quickly implement a POC of this myself. I realized it wasn't as easy as I thought because libpq doesn't actually expose an interface to send a Close message. So I submitted a patch for the next postgres commit fest to do add that functionality to libp: https://www.postgresql.org/message-id/flat/CAMsr%2BYEsk0z6rctRiw54CkAehosPternSdrws%2Bf8LKMd4b4MHA%40mail.gmail.com#c29d117e648ef4fdf621f87a664dfd74

https://commitfest.postgresql.org/43/4373/

[dvarrazzo]

If it's merged, no problem using it in place of DEALLOCATE.

[JelteF]

It's merged now (for PG17): postgres/postgres@28b5726

So can this issue be re-opened?

[dvarrazzo]

Cool! Are you able to provide a MR to use it in psycopg?

[JelteF]

Can this issue be re-opened?

[dvarrazzo]

Hi @JelteF! Sorry, I missed the action when you wrote back in July.