From: | Sami Imseih <samimseih(at)gmail(dot)com> |
---|---|
To: | Mircea Cadariu <cadariu(dot)mircea(at)gmail(dot)com> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Frédéric Yhuel <frederic(dot)yhuel(at)dalibo(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Benoit Lobréau <benoit(dot)lobreau(at)dalibo(dot)com>, Guillaume Lelarge <guillaume(dot)lelarge(at)dalibo(dot)com>, Pierrick Chovelon <pierrick(dot)chovelon(at)dalibo(dot)com> |
Subject: | Re: [BUG] temporary file usage report with extended protocol and unnamed portals |
Date: | 2025-09-19 20:28:46 |
Message-ID: | CAA5RZ0tTrTUoEr3kDXCuKsvqYGq8OOHiBwoD-dyJocq95uEOTQ@mail.gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
>> This makes me wonder then if all it takes is just adding this to PortalDrop (proposed earlier in the thread by Frédéric):
> One thing I did not like about that approach is that we will need to
> save the current debug_query_string inside PortalDrop before
> temporarily setting it to the one from the about to be dropped
> portal, and then set it back to the saved one before exiting.
> Otherwise, we might end up logging the wrong query in some cases
> (although I could not find a test case that proves my worry).
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.
--
Sami
Attachment | Content-Type | Size |
---|---|---|
v13-0002-Add-tests-for-temp-file-logging.patch | application/octet-stream | 5.0 KB |
v13-0001-Fix-temp-file-logging-blame-in-extended-query.patch | application/octet-stream | 2.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Alexandra Wang | 2025-09-19 20:40:32 | Re: SQL:2023 JSON simplified accessor support |
Previous Message | Tom Lane | 2025-09-19 20:14:38 | Re: Having postgresql.org link to cgit instead of gitweb |