From: Mircea Cadariu <cadariu.mircea@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Frédéric Yhuel <frederic.yhuel@dalibo.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pgsql: Drop unnamed portal immediately after execution to completion
Date: Thu, 13 Nov 2025 12:26:19 +0000
Message-ID: <e0ab1ea2-7084-44c5-b6aa-f01db7be7792@gmail.com> (raw)
In-Reply-To: <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>
<aRV6D3vxMQTQ7CxG@paquier.xyz>
On 13/11/2025 06:27, Michael Paquier wrote:
> 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.
Thanks for having a look! FWIW I found a way for Plan C to work without
including tcoprot into fd, see attached.
There's a new field in that structure indeed, but maybe not that far
fetched, it's the query that triggered the creation of the file.
Kind regards,
Mircea Cadariu
Attachments:v2-0001-show-temp-file-creating-query.patch (text/plain, 7.9K) download
From 08e5e884a2d86e0510d29c94fdf17405a25d822c Mon Sep 17 00:00:00 2001
From: Mircea Cadariu <cadariu.mircea@gmail.com>
Date: Wed, 12 Nov 2025 10:45:21 +0000
Subject: [PATCH v1] show temp file creating query
---
src/backend/storage/file/fd.c | 39 ++++++++++++++++---
src/backend/tcop/postgres.c | 10 -----
src/include/miscadmin.h | 2 +
src/include/tcop/tcopprot.h | 1 -
.../modules/test_misc/t/009_log_temp_files.pl | 16 ++++----
5 files changed, 43 insertions(+), 25 deletions(-)
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.cindex a4ec7959f3..f03d31fbd1 100644--- a/src/backend/storage/file/fd.c+++ b/src/backend/storage/file/fd.c@@ -206,6 +206,7 @@ typedef struct vfd
/* NB: fileName is malloc'd, and must be free'd when closing the VFD */
int fileFlags; /* open(2) flags for (re)opening the file */
mode_t fileMode; /* mode to pass to open(2) */
+ char *temp_file_creator_query; /* creator query for this temp file, if any */
} Vfd;
/*
@@ -1482,6 +1483,11 @@ FreeVfd(File file)
free(vfdP->fileName);
vfdP->fileName = NULL;
}
+ if (vfdP->temp_file_creator_query != NULL)+ {+ free(vfdP->temp_file_creator_query);+ vfdP->temp_file_creator_query = NULL;+ }
vfdP->fdstate = 0x0;
vfdP->nextFree = VfdCache[0].nextFree;
@@ -1524,18 +1530,27 @@ FileAccess(File file)
/*
* Called whenever a temporary file is deleted to report its size.
+ * If temp_file_creator_query is non-NULL, it represents the query that created this+ * temp file and will be logged.
*/
static void
-ReportTemporaryFileUsage(const char *path, off_t size)+ReportTemporaryFileUsage(const char *path, off_t size, const char *temp_file_creator_query)
{
pgstat_report_tempfile(size);
if (log_temp_files >= 0)
{
if ((size / 1024) >= log_temp_files)
- ereport(LOG,- (errmsg("temporary file: path \"%s\", size %lu",- path, (unsigned long) size)));+ {+ if (temp_file_creator_query != NULL)+ ereport(LOG,+ (errmsg("temporary file: path \"%s\", size %lu, created due to: %s",+ path, (unsigned long) size, temp_file_creator_query)));+ else+ ereport(LOG,+ (errmsg("temporary file: path \"%s\", size %lu",+ path, (unsigned long) size)));+ }
}
}
@@ -1842,6 +1857,12 @@ OpenTemporaryFileInTablespace(Oid tblspcOid, bool rejectError)
tempfilepath);
}
+ /*+ * Remember the creator query for this temp file.+ */+ if (file > 0 && debug_query_string != NULL)+ VfdCache[file].temp_file_creator_query = strdup(debug_query_string);+
return file;
}
@@ -1889,6 +1910,12 @@ PathNameCreateTemporaryFile(const char *path, bool error_on_failure)
/* Register it for automatic close. */
RegisterTemporaryFile(file);
+ /*+ * Remember the creator query for this temp file.+ */+ if (debug_query_string != NULL)+ VfdCache[file].temp_file_creator_query = strdup(debug_query_string);+
return file;
}
@@ -1960,7 +1987,7 @@ PathNameDeleteTemporaryFile(const char *path, bool error_on_failure)
}
if (stat_errno == 0)
- ReportTemporaryFileUsage(path, filestats.st_size);+ ReportTemporaryFileUsage(path, filestats.st_size, NULL);
else
{
errno = stat_errno;
@@ -2048,7 +2075,7 @@ FileClose(File file)
/* and last report the stat results */
if (stat_errno == 0)
- ReportTemporaryFileUsage(vfdP->fileName, filestats.st_size);+ ReportTemporaryFileUsage(vfdP->fileName, filestats.st_size, vfdP->temp_file_creator_query);
else
{
errno = stat_errno;
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.cindex 2bd8910268..7dd75a490a 100644--- a/src/backend/tcop/postgres.c+++ b/src/backend/tcop/postgres.c@@ -2327,16 +2327,6 @@ exec_execute_message(const char *portal_name, long max_rows)
* message. The next protocol message will start a fresh timeout.
*/
disable_statement_timeout();
-- /*- * We completed fetching from an unnamed portal. There is no need- * for it beyond this point, so drop 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.- */- if (portal->name[0] == '\0')- PortalDrop(portal, false);
}
/* Send appropriate CommandComplete to client */
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.hindex 9a7d733dde..030ad593a8 100644--- a/src/include/miscadmin.h+++ b/src/include/miscadmin.h@@ -208,6 +208,8 @@ extern PGDLLIMPORT Oid MyDatabaseId;
extern PGDLLIMPORT Oid MyDatabaseTableSpace;
+extern PGDLLIMPORT const char *debug_query_string;+
extern PGDLLIMPORT bool MyDatabaseHasLoginEventTriggers;
/*
diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.hindex c1bcfdec67..962eec7f9b 100644--- a/src/include/tcop/tcopprot.h+++ b/src/include/tcop/tcopprot.h@@ -23,7 +23,6 @@
typedef struct ExplainState ExplainState; /* defined in explain_state.h */
extern PGDLLIMPORT CommandDest whereToSendOutput;
-extern PGDLLIMPORT const char *debug_query_string;
extern PGDLLIMPORT int PostAuthDelay;
extern PGDLLIMPORT int client_connection_check_interval;
diff --git a/src/test/modules/test_misc/t/009_log_temp_files.pl b/src/test/modules/test_misc/t/009_log_temp_files.plindex 7ecd301ae2..4d88577e98 100644--- a/src/test/modules/test_misc/t/009_log_temp_files.pl+++ b/src/test/modules/test_misc/t/009_log_temp_files.pl@@ -39,7 +39,7 @@ SELECT 'unnamed portal';
END;
});
ok( $node->log_contains(
- qr/LOG:\s+temporary file: path.*\n.*\ STATEMENT:\s+SELECT a FROM foo ORDER BY a OFFSET \$1/s,+ qr/LOG:\s+temporary file: path.*created due to: SELECT a FROM foo ORDER BY a OFFSET \$1/s,
$log_offset),
"unnamed portal");
@@ -51,7 +51,7 @@ $node->safe_psql(
SELECT a FROM foo ORDER BY a OFFSET \$1 \\bind 4991 \\g
});
ok( $node->log_contains(
- qr/LOG:\s+temporary file: path.*\n.*\ STATEMENT:\s+SELECT a FROM foo ORDER BY a OFFSET \$1/s,+ qr/LOG:\s+temporary file: path.*created due to: SELECT a FROM foo ORDER BY a OFFSET \$1/s,
$log_offset),
"bind and implicit transaction");
@@ -65,7 +65,7 @@ SELECT 'named portal';
END;
});
ok( $node->log_contains(
- qr/LOG:\s+temporary file: path.*\n.*\ STATEMENT:\s+SELECT a FROM foo ORDER BY a OFFSET \$1/s,+ qr/LOG:\s+temporary file: path.*created due to: SELECT a FROM foo ORDER BY a OFFSET \$1/s,
$log_offset),
"named portal");
@@ -79,7 +79,7 @@ SELECT 'pipelined query';
\\endpipeline
});
ok( $node->log_contains(
- qr/LOG:\s+temporary file: path.*\n.*\ STATEMENT:\s+SELECT a FROM foo ORDER BY a OFFSET \$1/s,+ qr/LOG:\s+temporary file: path.*created due to: SELECT a FROM foo ORDER BY a OFFSET \$1/s,
$log_offset),
"pipelined query");
@@ -91,7 +91,7 @@ SELECT a, a, a FROM foo ORDER BY a OFFSET \$1 \\parse p1
\\bind_named p1 4993 \\g
});
ok( $node->log_contains(
- qr/LOG:\s+temporary file: path.*\n.*\ STATEMENT:\s+SELECT a, a, a FROM foo ORDER BY a OFFSET \$1/s,+ qr/LOG:\s+temporary file: path.*created due to: SELECT a, a, a FROM foo ORDER BY a OFFSET \$1/s,
$log_offset),
"parse and bind");
@@ -104,7 +104,7 @@ SELECT a FROM foo ORDER BY a OFFSET 4994;
END;
});
ok( $node->log_contains(
- qr/LOG:\s+temporary file: path.*\n.*\ STATEMENT:\s+SELECT a FROM foo ORDER BY a OFFSET 4994;/s,+ qr/LOG:\s+temporary file: path.*created due to: SELECT a FROM foo ORDER BY a OFFSET 4994;/s,
$log_offset),
"simple query");
@@ -120,7 +120,7 @@ CLOSE mycur;
END;
});
ok( $node->log_contains(
- qr/LOG:\s+temporary file: path.*\n.*\ STATEMENT:\s+CLOSE mycur;/s,+ qr/LOG:\s+temporary file: path.*created due to: FETCH 10 FROM mycur;/s,
$log_offset),
"cursor");
@@ -135,7 +135,7 @@ DEALLOCATE p1;
END;
});
ok( $node->log_contains(
- qr/LOG:\s+temporary file: path.*\n.*\ STATEMENT:\s+EXECUTE p1;/s,+ qr/LOG:\s+temporary file: path.*created due to: EXECUTE p1;/s,
$log_offset),
"prepare/execute");
--
2.39.5 (Apple Git-154)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: cadariu.mircea@gmail.com, michael@paquier.xyz, robertmhaas@gmail.com, tgl@sss.pgh.pa.us, frederic.yhuel@dalibo.com, pgsql-hackers@lists.postgresql.org
Subject: Re: pgsql: Drop unnamed portal immediately after execution to completion
In-Reply-To: <e0ab1ea2-7084-44c5-b6aa-f01db7be7792@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox