On Fri, Sep 19, 2025 at 03:28:46PM -0500, Sami Imseih wrote:
> After thinking about this a bit more, I found the test that breaks
> with v12. It is a bind statement in an implicit transaction. The
> portal will get dropped by the end of the transaction and will not
> reach drop_unnamed_portal. So, v13 takes Frederic's original idea,
> saves the pointer of debug_query_string, and resets it at the end of
> DropPortal. I also enhanced the test coverage.
>
> Debugging also shows that the STATEMENT: log is formed while we are
> in ExecutorEnd. We know that other extensions rely on
> QueryDesc->sourceText in that hook (i.e., pg_stat_statements), so
> this is another reason this approach appears safe, in addition to
> what was mentioned here [0] about the MessageContext outliving the
> portal.
>
> [0] https://www.postgresql.org/message-id/CAA5RZ0vB-h2pFimPSi72ObWfpRwKR5kaN9XWW17TOkLntC9svA%40mail.gmail.com
Hmm. We are running in circles here, based on the argument that the
drop of an unnamed portal happens when the next BIND command shows
up, and the fact that it is an old protocol artifact that we have been
leaving with for ages in the backend engine for efficiency.
I do agree about the point that it may be better to encourage drivers
to not use unnamed portals, but at the same time I don't recall
anybody complaining about the fact that a statement string should
absolutely match with the statement where a temporary file has been
created and should be linked to it.
Thinking about this problem in a twisted way, could there be an
argument in favor of the existing logic as it is? It is true that the
cleanup happens when the next bind query happens. So, in fact, one
could also say that it makes sense to reflect when a temp file is
cleaned up and what's the query being processed that does the cleanup.
In this case, it is not the query that created the temp file, but the
one that's been processed, and the portal drop is documented in
protocol.sgml as being part of the follow-up BIND. (I did use the
term "twisted" here.)
In short, I would be inclined to do nothing here, but I do see an
argument in favor of the tests, particularly to track the fact that
the unnamed portal drop happens in the next BIND query, and looking
for temp file getting dropped in the server logs is an interesting way
to validate that. Some of the proposals of this thread broke this
protocol item, so that seems at least important to track in the
long-term.
--
Michael
