Re: [BUG] temporary file usage report with extended protocol and unnamed portals - Mailing list pgsql-hackers

From Sami Imseih
Subject Re: [BUG] temporary file usage report with extended protocol and unnamed portals
Date
Msg-id CAA5RZ0tTrTUoEr3kDXCuKsvqYGq8OOHiBwoD-dyJocq95uEOTQ@mail.gmail.com
Whole thread Raw
In response to Re: [BUG] temporary file usage report with extended protocol and unnamed portals  (Sami Imseih <samimseih@gmail.com>)
List 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.

[0] https://www.postgresql.org/message-id/CAA5RZ0vB-h2pFimPSi72ObWfpRwKR5kaN9XWW17TOkLntC9svA%40mail.gmail.com

--
Sami

Attachment

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Having postgresql.org link to cgit instead of gitweb
Next
From: Alexandra Wang
Date:
Subject: Re: SQL:2023 JSON simplified accessor support