Re: pgsql: Drop unnamed portal immediately after execution to completion

On Mon, Nov 10, 2025 at 6:31 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> All the other solutions mentioned mean to work around the issue of the
> unnamed portal still being present by maintaining the query string it
> in a different context across multiple messages. And I doubt that
> anybody would be thrilled by that.

Why not?

I mean, I haven't studied this problem and I don't know how
complicated that would be, but it isn't obvious to me that it would be
wildly impractical. We would need to pass around pointers to the query
string, and make sure that the string itself isn't freed before all
the pointers are gone, but maybe there's a reasonable way to do that.
I would definitely rather do that than change the wire protocol. If
that isn't practical, another option might be to just suppress the
context in places where we know it can be misleading, rather than
displaying a misleading context. That would still require us to know
in which situations that the context might be misleading, which would
likely merit some careful thought, but it would at least avoid us
needing to know the query string upon which we wanted to place blame.
Of course, this approach would produce less useful output, but it
would still be better than showing wrong information. (I see that Tom
previously suggested exactly this approach.)

To me, this is a classic example of why programming with global
variables is often a bad idea. If we lived in a world where a backend
could only ever do one thing at a time, then consulting a global
variable to find out what it's currently doing would be fine, but we
have this whole system of portals and prepared statements precisely so
that a backend can do more than one thing at a time, so the right
thing to do is pass around pointers to the appropriate context object
-- a Portal, or whatever -- to all the code that needs that
information. If the existing context object, a Portal or whatever,
doesn't have a long enough lifetime to still be available at all
points where we need that information, then we should either make the
existing type of context object live longer or invent a separate kind
of context object that with a different lifespan that is suited to the
remaining problems. Given how old and crufty the Portal machinery
seems to be, the latter seems like the likely winner.

> If you think that we should continue to live with the protocol for
> unnamed portals as-is and continue to live with the existing behavior,
> meaning a revert of 1fd981f05369, that would not be the end of the
> world here: we'd still have some tests that track the
> past-and-currently-released behavior.

Yes, I'd argue for a revert. I think the risk of subtle breakage is
high, and there is a good chance that it will be too late to pull this
back by the time we realize what the problem is. I also feel that it's
solving the problem in the wrong way. To me, this solution feels like
saying "doing proper bookkeeping is hard, so let's redefine the
contract with the user instead." But doing proper bookkeeping is a big
part of what good software engineering is all about. We've put an
enormous amount of energy into our error reporting and a lot of that
energy has gone into making sure that the details that would be useful
to the user are available in the parts of the code that need those
details. There's lots and lots of code already getting query strings
to be available at places where we might want to complain about them,
and very often we also pass through a parse location as well, so that
we can complain about a specific part of a query string. The fact that
the query string that shows up in this case is wrong or misleading
doesn't make that the wrong approach; it just means we haven't totally
solved the problem yet.

> Even if strange, it works with the timing where the unnamed protocol
> is dropped. Perhaps somebody would be able to come up with a approach
> better than a more aggressive portal drop once completed, but I don't
> quite see how much can be done as long as we manipulate the unnamed
> portals this way (aka drop then with a follow-up bind, and expect the
> logging to show the correct statements attached to a previous
> message that we have no knowledge about). The original statement
> logging did not really consider all these fancier cases with the
> extended query protocol.

I have a really hard time with the idea that the problem here is with
when the unnamed portal is dropped. The reasoning behind the existing
wire protocol specification is a little unclear to me, but it seems
like a perfectly elegant concept: you keep the unnamed portal around
until something creates a new unnamed portal. I like that definition
because it's simply and devoid of ambiguity, so other software
interacting with PostgreSQL knows exactly what to expect. When you
start to make the rules more complicated, that makes it harder for
other software that speaks our wire protocol to be fully correct,
especially if it must deal with both an old rule and a new one
depending on the version of PostgreSQL to which it's connected. I
think you could make an argument here that the problem is with when
the temporary file cleanup is done (perhaps it should happen sooner,
when the previous query is still in context) or that the temporary
file cleanup should be passed some appropriate context so that it
knows who to blame for error or that the temporary file cleanup can't
really attribute any errors to a specific statement and thus shouldn't
mention one at all, but I don't think blaming the unnamed portal for
this is the right idea. Our portal management code seems to me to be
full of deficiencies, but the wire protocol specification itself seems
to be fine, so I think we should try to do a better job implementing
the protocol, rather than changing it.

--
Robert Haas
EDB: http://www.enterprisedb.com