Re: [BUG] temporary file usage report with extended protocol and unnamed portals

From: Sami Imseih <samimseih(at)gmail(dot)com>
To: Frédéric Yhuel <frederic(dot)yhuel(at)dalibo(dot)com>
Cc: 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-04-23 16:13:17
Message-ID: CAA5RZ0tzLmZXbucPsp7-MW0u9zT-NV5drq22MDYkYC2nngS4Sg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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)

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nathan Bossart 2025-04-23 16:18:20 Re: Support NOT VALID / VALIDATE constraint options for named NOT NULL constraints
Previous Message Robert Haas 2025-04-23 16:12:11 Re: pgsql: Add function to get memory context stats for processes