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 CAA5RZ0tzLmZXbucPsp7-MW0u9zT-NV5drq22MDYkYC2nngS4Sg@mail.gmail.com
Whole thread Raw
In response to Re: [BUG] temporary file usage report with extended protocol and unnamed portals  (Frédéric Yhuel <frederic.yhuel@dalibo.com>)
Responses Re: [BUG] temporary file usage report with extended protocol and unnamed portals
List pgsql-hackers
> Yes, I think I had misunderstood what Tom said. Thank you for pointing
> that out.
>
> However, is it really unsafe?

I have not been able to find a case where this is unsafe, but
the documentation in mmgr/README does indicate that the
query string may live shorter than the portal in some cases.

"....
This
is kept separate from per-transaction and per-portal contexts because a
query string might need to live either a longer or shorter time than any
single transaction or portal.
"""

Also, another strange behavior of the way portal cleanup occurs is that
in extended-query-protocol and within a transaction, ExecutorEnd for the
last query is not actually called until the next command. This just seems
odd to me especially for extensions that rely on ExecutorEnd.

So, Can we do something like this? This drops the portal as soon as
execution completes ( the portal is fetched to completion ). This will
ensure that there is no delay in ExecutorEnd getting called and in the
case of log_temp_files, the message will be logged while debug_query_string
is still pointing to the correct query.


diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index dc4c600922d..efe0151ca8f 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2327,6 +2327,9 @@ exec_execute_message(const char *portal_name,
long max_rows)

                /* Send appropriate CommandComplete to client */
                EndCommand(&qc, dest, false);
+
+               if (!portal->portalPinned)
+                       PortalDrop(portal, false);
        }
        else
        {



```
postgres=# begin;
BEGIN
postgres=*# select from foo order by a \bind
postgres-*# ;
--
(1000000 rows)

postgres=*# select 1 \bind
postgres-*# ;
 ?column?
----------
        1
(1 row)

postgres=*# commit;
COMMIT
```

```
2025-04-23 11:11:47.777 CDT [67362] LOG:  temporary file: path
"base/pgsql_tmp/pgsql_tmp67362.0", size 1009983488
2025-04-23 11:11:47.777 CDT [67362] STATEMENT:  select from foo order by a
    ;
```


thoughts?

--
Sami Imseih
Amazon Web Services (AWS)



pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: pgsql: Add function to get memory context stats for processes
Next
From: Nathan Bossart
Date:
Subject: Re: Support NOT VALID / VALIDATE constraint options for named NOT NULL constraints