Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 13 Nov 2025 15:27:23 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Mircea Cadariu <cadariu.mircea@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
	=?iso-8859-1?Q?Fr=E9d=E9ric?= Yhuel <frederic.yhuel@dalibo.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pgsql: Drop unnamed portal immediately after execution to
 completion
Message-ID: <aRV6D3vxMQTQ7CxG@paquier.xyz>
References: <E1vGWIt-0056al-2N@gemulon.postgresql.org>
 <CA+TgmoYFJyJNQw3RT7veO3M2BWRE9Aw4hprC5rOcawHZti-f8g@mail.gmail.com>
 <1982325.1762810082@sss.pgh.pa.us>
 <aRJ1zn0z-Bn0qc2H@paquier.xyz>
 <CA+TgmoYR2OP8C+MHC8VCA80-r=Q09aF-_=juvBLvUKS09d__Mw@mail.gmail.com>
 <00d71a54-2118-4b18-87e8-33f4a99dc624@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="VRlUH6Ha4FDi4uJe"
Content-Disposition: inline
In-Reply-To: <00d71a54-2118-4b18-87e8-33f4a99dc624@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/aRV6D3vxMQTQ7CxG%40paquier.xyz>
Precedence: bulk


--VRlUH6Ha4FDi4uJe
Content-Type: multipart/mixed; boundary="ymSkWWb2wMYYhose"
Content-Disposition: inline


--ymSkWWb2wMYYhose
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Nov 12, 2025 at 11:14:26AM +0000, Mircea Cadariu wrote:
> I understand the point that=E2=80=99s been raised. Would it be an option =
to indeed
> revert the portal drop in postgres.c, but then append the right query in =
the
> "temporary file: ..." log line instead? This would work at least for me.

This is new, attaching the information to a Vfd in fd.c.  Not sure
that adding this information to this structure is a good concept.
This layer of the code has no idea of query strings currently, so that
feels a bit like a layer violation.

Thinking a bit outside the box..  I was wondering about a plan D (plan
A is what's on HEAD, plan B is copying around the query string, plan C
this Vfd approach) where we shut down the executor when we have
finished the execution of an unnamed portal, with something like the
attached based on a more aggressive PortalCleanup().  However, that
would not fly far if the client decides to send an extra execute
message post-portal-completion where we'd still want the executor to
be around, even if there are no rows to process.  We presumably do not
need the temp files anymore at this stage, but I don't really like the
fact that we'd need to somehow take a shortcut if we want only to
clean up the temp files.

Perhaps the best answer for now is to revert and continue the
discussion for this cycle as there seem to be little love for the
current HEAD's solution with the protocol change.

If folks have more ideas or input, please feel free.
--
Michael

--ymSkWWb2wMYYhose
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="portal-cleanup.patch"
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 2bd89102686e..8d3739298afe 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -38,6 +38,7 @@
 #include "commands/async.h"
 #include "commands/event_trigger.h"
 #include "commands/explain_state.h"
+#include "commands/portalcmds.h"
 #include "commands/prepare.h"
 #include "common/pg_prng.h"
 #include "jit/jit.h"
@@ -2330,13 +2331,14 @@ exec_execute_message(const char *portal_name, long =
max_rows)
=20
 			/*
 			 * We completed fetching from an unnamed portal.  There is no need
-			 * for it beyond this point, so drop it now rather than wait for
+			 * for it beyond this point, so clean it now rather than wait for
 			 * the next Bind message to do this cleanup.  This ensures that
 			 * the correct statement is logged when cleaning up temporary file
-			 * usage.
+			 * usage with the executor shutdown.
 			 */
-			if (portal->name[0] =3D=3D '\0')
-				PortalDrop(portal, false);
+			if (portal->name[0] =3D=3D '\0' &&
+				portal->cleanup)
+				PortalCleanup(portal);
 		}
=20
 		/* Send appropriate CommandComplete to client */

--ymSkWWb2wMYYhose--

--VRlUH6Ha4FDi4uJe
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmkVeksACgkQnvQgOdby
QH3nwQ//cWaV1jXDpDhwrIUkYeoPedEqz+cPC0mw26FgcGxSp/dreqBzZiow2+27
NFg5vWOzD6Ra2F9DKsTcbuUyjcGvBvE/QR0whgSUfw4TzkwFVXaMRHkqsNM3m5fl
TgmBoA3RezZyp7yERmrOmtyQGIN03alKLlrFLNhWOyCEntMskC52OSAIfjylK3zt
zrxUZzcupDNJcwT7ST+V9Pd9FdogXq9/mH0mn/JOAEu5awI84j8XGFPJ1jbFhbbL
puzsgxKCOuAWDx604fnbdupaiFi82OXPsZ/kmoxKaOhKE5pWDE/JVyUY/hiBzmYn
iPElxLz7z2eYpTbuHjpXguA0liv5e+XYx2Gx3I+qBWvORYY5YY6l2OtY6nwxuCUf
yzY1RV7Vg6nCIExcdvWaHsXg6Whvf2XsGA3HDG3V28VLtqabBZSSIvTf0L4GpPZt
19GEWpoXACfxjPUhqCeX/ofc+PeIodZixDXykhbzKYKwfKNbDkwDea7fLtqhMsWi
Q4+tzhaokL++TsfgWlRsZXZJfp6mkShdUfS4OeowXtfWQ5sC6h/p9gqUJjeFiI+G
AmJ0c9813AzPQ6Yt7IJ/XN0rgnkqMmdy7v/whVitN/jkKxx31pQH9ZUmbndNT4FU
sWA8p5XEOyjcdDMe20HL7YyO0q9nJxsn2lfWwzRRFWVr88a0uQ0=
=3a7w
-----END PGP SIGNATURE-----

--VRlUH6Ha4FDi4uJe--