Re: [BUG] temporary file usage report with extended protocol and unnamed portals
От | Michael Paquier |
---|---|
Тема | Re: [BUG] temporary file usage report with extended protocol and unnamed portals |
Дата | |
Msg-id | aNXqU0EHiMD_W62J@paquier.xyz обсуждение исходный текст |
Ответ на | Re: [BUG] temporary file usage report with extended protocol and unnamed portals (Sami Imseih <samimseih@gmail.com>) |
Ответы |
Re: [BUG] temporary file usage report with extended protocol and unnamed portals
Re: [BUG] temporary file usage report with extended protocol and unnamed portals |
Список | pgsql-hackers |
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
Вложения
В списке pgsql-hackers по дате отправления: