pg_ls_tmpdir()

Started by Bossart, Nathanover 7 years ago14 messages
#1Bossart, Nathan
bossartn@amazon.com
1 attachment(s)

Hi hackers,

Attached is a patch to add a pg_ls_tmpdir() function that lists the
contents of a specified tablespace's pgsql_tmp directory. This is
very similar to the existing pg_ls_logdir() and pg_ls_waldir()
functions.

By providing more visibility into the temporary file directories,
users can more easily track queries that are consuming a lot of disk
space for temporary files. This function already provides enough
information to calculate the total temporary file usage per PID, but
it might be worthwhile to create a system view that does this, too.

I am submitting this patch for consideration in commitfest 2018-11.

Nathan

Attachments:

0001-Add-pg_ls_tmpdir-function.patchapplication/octet-stream; name=0001-Add-pg_ls_tmpdir-function.patchDownload
From 7a1e2cd321ce9e5393a96e6bf4ff0267efea0e25 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <bossartn@amazon.com>
Date: Wed, 12 Sep 2018 20:09:37 +0000
Subject: [PATCH 1/1] Add pg_ls_tmpdir() function.

---
 doc/src/sgml/func.sgml               | 27 +++++++++++++++++++++++++++
 src/backend/catalog/system_views.sql |  4 ++++
 src/backend/utils/adt/genfile.c      | 33 +++++++++++++++++++++++++++++++++
 src/include/catalog/catversion.h     |  2 +-
 src/include/catalog/pg_proc.dat      | 10 ++++++++++
 5 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 4331bebc96..83f8e39f5a 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -20220,6 +20220,20 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
         role and may be granted to other non-superuser roles.
        </entry>
       </row>
+      <row>
+       <entry>
+        <literal><function>pg_ls_tmpdir(<optional><parameter>tablespace</parameter> <type>oid</type></optional>)</function></literal>
+       </entry>
+       <entry><type>setof record</type></entry>
+       <entry>
+        List the name, size, and last modification time of files in the
+        temporary directory for <parameter>tablespace</parameter>.  If
+        <parameter>tablespace</parameter> is not provided, the
+        <literal>pg_default</literal> tablespace is used.  Access is granted to
+        members of the <literal>pg_monitor</literal> role and may be granted to
+        other non-superuser roles.
+       </entry>
+      </row>
       <row>
        <entry>
         <literal><function>pg_read_file(<parameter>filename</parameter> <type>text</type> [, <parameter>offset</parameter> <type>bigint</type>, <parameter>length</parameter> <type>bigint</type> [, <parameter>missing_ok</parameter> <type>boolean</type>] ])</function></literal>
@@ -20293,6 +20307,19 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
     <command>GRANT</command>.
    </para>
 
+   <indexterm>
+    <primary>pg_ls_tmpdir</primary>
+   </indexterm>
+   <para>
+    <function>pg_ls_tmpdir</function> returns the name, size, and last modified
+    time (mtime) of each file in the temporary file directory for the specified
+    <parameter>tablespace</parameter>.  If <parameter>tablespace</parameter> is
+    not provided, the <literal>pg_default</literal> tablespace is used.  By
+    default only superusers and members of the <literal>pg_monitor</literal>
+    role can use this function.  Access may be granted to others using
+    <command>GRANT</command>.
+   </para>
+
    <indexterm>
     <primary>pg_read_file</primary>
    </indexterm>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 7251552419..020f28cbf6 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1150,6 +1150,8 @@ REVOKE EXECUTE ON FUNCTION lo_export(oid, text) FROM public;
 
 REVOKE EXECUTE ON FUNCTION pg_ls_logdir() FROM public;
 REVOKE EXECUTE ON FUNCTION pg_ls_waldir() FROM public;
+REVOKE EXECUTE ON FUNCTION pg_ls_tmpdir() FROM public;
+REVOKE EXECUTE ON FUNCTION pg_ls_tmpdir(oid) FROM public;
 
 REVOKE EXECUTE ON FUNCTION pg_read_file(text) FROM public;
 REVOKE EXECUTE ON FUNCTION pg_read_file(text,bigint,bigint) FROM public;
@@ -1170,6 +1172,8 @@ REVOKE EXECUTE ON FUNCTION pg_ls_dir(text,boolean,boolean) FROM public;
 --
 GRANT EXECUTE ON FUNCTION pg_ls_logdir() TO pg_monitor;
 GRANT EXECUTE ON FUNCTION pg_ls_waldir() TO pg_monitor;
+GRANT EXECUTE ON FUNCTION pg_ls_tmpdir() TO pg_monitor;
+GRANT EXECUTE ON FUNCTION pg_ls_tmpdir(oid) TO pg_monitor;
 
 GRANT pg_read_all_settings TO pg_monitor;
 GRANT pg_read_all_stats TO pg_monitor;
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index a97cbea248..686207417a 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -23,6 +23,7 @@
 #include "access/htup_details.h"
 #include "access/xlog_internal.h"
 #include "catalog/pg_authid.h"
+#include "catalog/pg_tablespace_d.h"
 #include "catalog/pg_type.h"
 #include "funcapi.h"
 #include "mb/pg_wchar.h"
@@ -610,3 +611,35 @@ pg_ls_waldir(PG_FUNCTION_ARGS)
 {
 	return pg_ls_dir_files(fcinfo, XLOGDIR);
 }
+
+/*
+ * Generic function to return the list of files in pgsql_tmp
+ */
+static Datum
+pg_ls_tmpdir(FunctionCallInfo fcinfo, Oid tblspc)
+{
+	char		path[MAXPGPATH];
+
+	TempTablespacePath(path, tblspc);
+	return pg_ls_dir_files(fcinfo, path);
+}
+
+/*
+ * Function to return the list of temporary files in the pg_default tablespace's
+ * pgsql_tmp directory
+ */
+Datum
+pg_ls_tmpdir_noargs(PG_FUNCTION_ARGS)
+{
+	return pg_ls_tmpdir(fcinfo, DEFAULTTABLESPACE_OID);
+}
+
+/*
+ * Function to return the list of temporary files in the specified tablespace's
+ * pgsql_tmp directory
+ */
+Datum
+pg_ls_tmpdir_1arg(PG_FUNCTION_ARGS)
+{
+	return pg_ls_tmpdir(fcinfo, PG_GETARG_OID(0));
+}
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index f898a2225f..595921d672 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
  */
 
 /*							yyyymmddN */
-#define CATALOG_VERSION_NO	201809052
+#define CATALOG_VERSION_NO	201809121
 
 #endif
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 860571440a..279c8236bd 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10199,6 +10199,16 @@
   provolatile => 'v', prorettype => 'record', proargtypes => '',
   proallargtypes => '{text,int8,timestamptz}', proargmodes => '{o,o,o}',
   proargnames => '{name,size,modification}', prosrc => 'pg_ls_waldir' },
+{ oid => '5029', descr => 'list files in the pgsql_tmp directory',
+  proname => 'pg_ls_tmpdir', procost => '10', prorows => '20', proretset => 't',
+  provolatile => 'v', prorettype => 'record', proargtypes => '',
+  proallargtypes => '{text,int8,timestamptz}', proargmodes => '{o,o,o}',
+  proargnames => '{name,size,modification}', prosrc => 'pg_ls_tmpdir_noargs' },
+{ oid => '5030', descr => 'list files in the pgsql_tmp directory',
+  proname => 'pg_ls_tmpdir', procost => '10', prorows => '20', proretset => 't',
+  provolatile => 'v', prorettype => 'record', proargtypes => 'oid',
+  proallargtypes => '{oid,text,int8,timestamptz}', proargmodes => '{i,o,o,o}',
+  proargnames => '{tablespace,name,size,modification}', prosrc => 'pg_ls_tmpdir_1arg' },
 
 # hash partitioning constraint function
 { oid => '5028', descr => 'hash partition CHECK constraint',
-- 
2.16.2

#2Laurenz Albe
laurenz.albe@cybertec.at
In reply to: Bossart, Nathan (#1)
Re: pg_ls_tmpdir()

Bossart, Nathan wrote:

Attached is a patch to add a pg_ls_tmpdir() function that lists the
contents of a specified tablespace's pgsql_tmp directory. This is
very similar to the existing pg_ls_logdir() and pg_ls_waldir()
functions.

By providing more visibility into the temporary file directories,
users can more easily track queries that are consuming a lot of disk
space for temporary files. This function already provides enough
information to calculate the total temporary file usage per PID, but
it might be worthwhile to create a system view that does this, too.

I am submitting this patch for consideration in commitfest 2018-11.

The patch applies, builds without warning and passes "make check-world".

Since pgsql_tmp is only created when the first temp file is written,
calling the function on a newly initdb'ed data directory fails with

ERROR: could not open directory "base/pgsql_tmp": No such file or directory

This should be fixed; it shouldn't be an error.

Calling the function with a non-existing tablespace OID produces:

ERROR: could not open directory "pg_tblspc/1665/PG_12_201809121/pgsql_tmp": No such file or directory

Wouldn't it be better to have a check if the tablespace exists?

About the code:
The catversion bump shouldn't be part of the patch, it will
rot too quickly. The committer is supposed to add it.

I think the idea to have such a function is fine, but I have two doubts:

1. Do we really need two functions, one without input argument
to list the default tablespace?
I think that anybody who uses with such a function whould
be able to feed the OID of "pg_default".

2. There already are functions "pg_ls_logdir" and "pg_ls_waldir",
and I can imagine new requests, e.g. pg_ls_datafiles() to list the
data files in a database directory.

It may make sense to have a generic function like

pg_ls_dir(dirname text, tablespace oid)

instead. But maybe that's taking it too far...

I'll set the patch to "waiting for author".

Yours,
Laurenz Albe

#3Justin Pryzby
pryzby@telsasoft.com
In reply to: Laurenz Albe (#2)
Re: pg_ls_tmpdir(); AND, Function for listing archive_status directory

On Wed, Sep 26, 2018 at 10:36:03PM +0200, Laurenz Albe wrote:

Bossart, Nathan wrote:

Attached is a patch to add a pg_ls_tmpdir() function that lists the
contents of a specified tablespace's pgsql_tmp directory. This is
very similar to the existing pg_ls_logdir() and pg_ls_waldir()
functions.

On Sun, Sep 30, 2018 at 10:59:20PM +0200, Christoph Moench-Tegeder wrote:

while setting up monitoring for a new PostgreSQL instance, I noticed that
there's no build-in way for a pg_monitor role to check the contents of
the archive_status directory. We got pg_ls_waldir() in 10, but that
only lists pg_wal - not it's (sic) subdirectory.

It may make sense to have a generic function like
pg_ls_dir(dirname text, tablespace oid)
instead. But maybe that's taking it too far...

I see Cristoph has another pg_ls_* function, which suggests that Laurenz idea
is good, and a generic pg_ls function is desirable.

Justin

#4Andres Freund
andres@anarazel.de
In reply to: Laurenz Albe (#2)
Re: pg_ls_tmpdir()

Hi,

On 2018-09-26 22:36:03 +0200, Laurenz Albe wrote:

2. There already are functions "pg_ls_logdir" and "pg_ls_waldir",
and I can imagine new requests, e.g. pg_ls_datafiles() to list the
data files in a database directory.

It may make sense to have a generic function like

pg_ls_dir(dirname text, tablespace oid)

instead. But maybe that's taking it too far...

There's a generic pg_ls_dir() already - I'm now sure why a generic
function would take the tablespace oid however?

Greetings,

Andres Freund

#5Bossart, Nathan
bossartn@amazon.com
In reply to: Andres Freund (#4)
1 attachment(s)
Re: pg_ls_tmpdir()

Hi,

On 9/26/18, 3:36 PM, "Laurenz Albe" <laurenz.albe@cybertec.at> wrote:

The patch applies, builds without warning and passes "make check-world".

Thanks for the review!

Since pgsql_tmp is only created when the first temp file is written,
calling the function on a newly initdb'ed data directory fails with

ERROR: could not open directory "base/pgsql_tmp": No such file or directory

This should be fixed; it shouldn't be an error.

Done.

Calling the function with a non-existing tablespace OID produces:

ERROR: could not open directory "pg_tblspc/1665/PG_12_201809121/pgsql_tmp": No such file or directory

Wouldn't it be better to have a check if the tablespace exists?

Done.

About the code:
The catversion bump shouldn't be part of the patch, it will
rot too quickly. The committer is supposed to add it.

Removed.

I think the idea to have such a function is fine, but I have two doubts:

1. Do we really need two functions, one without input argument
to list the default tablespace?
I think that anybody who uses with such a function whould
be able to feed the OID of "pg_default".

That seems reasonable to me. I've removed the second version of the
function.

2. There already are functions "pg_ls_logdir" and "pg_ls_waldir",
and I can imagine new requests, e.g. pg_ls_datafiles() to list the
data files in a database directory.

It may make sense to have a generic function like

pg_ls_dir(dirname text, tablespace oid)

instead. But maybe that's taking it too far...

There is an existing pg_ls_dir() function that appears to be available
only to superusers. IMO it's also nice to break out specific "safe"
directories like this for other roles (e.g. pg_monitor).

Nathan

Attachments:

tmpdir_patch_v2.patchapplication/octet-stream; name=tmpdir_patch_v2.patchDownload
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 9a7f683658..c09c205f16 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -20355,6 +20355,18 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
         role and may be granted to other non-superuser roles.
        </entry>
       </row>
+      <row>
+       <entry>
+        <literal><function>pg_ls_tmpdir(<parameter>tablespace</parameter> <type>oid</type>)</function></literal>
+       </entry>
+       <entry><type>setof record</type></entry>
+       <entry>
+        List the name, size, and last modification time of files in the
+        temporary directory for <parameter>tablespace</parameter>.  Access is
+        granted to members of the <literal>pg_monitor</literal> role and may be
+        granted to other non-superuser roles.
+       </entry>
+      </row>
       <row>
        <entry>
         <literal><function>pg_read_file(<parameter>filename</parameter> <type>text</type> [, <parameter>offset</parameter> <type>bigint</type>, <parameter>length</parameter> <type>bigint</type> [, <parameter>missing_ok</parameter> <type>boolean</type>] ])</function></literal>
@@ -20428,6 +20440,17 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
     <command>GRANT</command>.
    </para>
 
+   <indexterm>
+    <primary>pg_ls_tmpdir</primary>
+   </indexterm>
+   <para>
+    <function>pg_ls_tmpdir</function> returns the name, size, and last modified
+    time (mtime) of each file in the temporary file directory for the specified
+    <parameter>tablespace</parameter>.  By default only superusers and members
+    of the <literal>pg_monitor</literal> role can use this function.  Access may
+    be granted to others using <command>GRANT</command>.
+   </para>
+
    <indexterm>
     <primary>pg_read_file</primary>
    </indexterm>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 7251552419..13e684d9c7 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1150,6 +1150,7 @@ REVOKE EXECUTE ON FUNCTION lo_export(oid, text) FROM public;
 
 REVOKE EXECUTE ON FUNCTION pg_ls_logdir() FROM public;
 REVOKE EXECUTE ON FUNCTION pg_ls_waldir() FROM public;
+REVOKE EXECUTE ON FUNCTION pg_ls_tmpdir(oid) FROM public;
 
 REVOKE EXECUTE ON FUNCTION pg_read_file(text) FROM public;
 REVOKE EXECUTE ON FUNCTION pg_read_file(text,bigint,bigint) FROM public;
@@ -1170,6 +1171,7 @@ REVOKE EXECUTE ON FUNCTION pg_ls_dir(text,boolean,boolean) FROM public;
 --
 GRANT EXECUTE ON FUNCTION pg_ls_logdir() TO pg_monitor;
 GRANT EXECUTE ON FUNCTION pg_ls_waldir() TO pg_monitor;
+GRANT EXECUTE ON FUNCTION pg_ls_tmpdir(oid) TO pg_monitor;
 
 GRANT pg_read_all_settings TO pg_monitor;
 GRANT pg_read_all_stats TO pg_monitor;
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index a97cbea248..d4d338ec78 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -23,6 +23,7 @@
 #include "access/htup_details.h"
 #include "access/xlog_internal.h"
 #include "catalog/pg_authid.h"
+#include "catalog/pg_tablespace_d.h"
 #include "catalog/pg_type.h"
 #include "funcapi.h"
 #include "mb/pg_wchar.h"
@@ -31,6 +32,7 @@
 #include "storage/fd.h"
 #include "utils/builtins.h"
 #include "utils/memutils.h"
+#include "utils/syscache.h"
 #include "utils/timestamp.h"
 
 typedef struct
@@ -520,7 +522,7 @@ pg_ls_dir_1arg(PG_FUNCTION_ARGS)
 
 /* Generic function to return a directory listing of files */
 static Datum
-pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir)
+pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok)
 {
 	FuncCallContext *funcctx;
 	struct dirent *de;
@@ -549,10 +551,18 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir)
 		fctx->dirdesc = AllocateDir(fctx->location);
 
 		if (!fctx->dirdesc)
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not open directory \"%s\": %m",
-							fctx->location)));
+		{
+			if (missing_ok && errno == ENOENT)
+			{
+				MemoryContextSwitchTo(oldcontext);
+				SRF_RETURN_DONE(funcctx);
+			}
+			else
+				ereport(ERROR,
+						(errcode_for_file_access(),
+						 errmsg("could not open directory \"%s\": %m",
+								fctx->location)));
+		}
 
 		funcctx->user_fctx = fctx;
 		MemoryContextSwitchTo(oldcontext);
@@ -601,12 +611,32 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir)
 Datum
 pg_ls_logdir(PG_FUNCTION_ARGS)
 {
-	return pg_ls_dir_files(fcinfo, Log_directory);
+	return pg_ls_dir_files(fcinfo, Log_directory, false);
 }
 
 /* Function to return the list of files in the WAL directory */
 Datum
 pg_ls_waldir(PG_FUNCTION_ARGS)
 {
-	return pg_ls_dir_files(fcinfo, XLOGDIR);
+	return pg_ls_dir_files(fcinfo, XLOGDIR, false);
+}
+
+/*
+ * Function to return the list of temporary files in the specified tablespace's
+ * pgsql_tmp directory
+ */
+Datum
+pg_ls_tmpdir(PG_FUNCTION_ARGS)
+{
+	Oid			spc_oid = PG_GETARG_OID(0);
+	char		path[MAXPGPATH];
+
+	if (!SearchSysCacheExists1(TABLESPACEOID, ObjectIdGetDatum(spc_oid)))
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_OBJECT),
+				 errmsg("tablespace with OID %u does not exist",
+						spc_oid)));
+
+	TempTablespacePath(path, spc_oid);
+	return pg_ls_dir_files(fcinfo, path, true);
 }
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 8e4145f42b..a18f5bf17c 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10199,6 +10199,11 @@
   provolatile => 'v', prorettype => 'record', proargtypes => '',
   proallargtypes => '{text,int8,timestamptz}', proargmodes => '{o,o,o}',
   proargnames => '{name,size,modification}', prosrc => 'pg_ls_waldir' },
+{ oid => '5029', descr => 'list files in the pgsql_tmp directory',
+  proname => 'pg_ls_tmpdir', procost => '10', prorows => '20', proretset => 't',
+  provolatile => 'v', prorettype => 'record', proargtypes => 'oid',
+  proallargtypes => '{oid,text,int8,timestamptz}', proargmodes => '{i,o,o,o}',
+  proargnames => '{tablespace,name,size,modification}', prosrc => 'pg_ls_tmpdir' },
 
 # hash partitioning constraint function
 { oid => '5028', descr => 'hash partition CHECK constraint',
#6Christoph Berg
myon@debian.org
In reply to: Bossart, Nathan (#5)
Re: pg_ls_tmpdir()

Re: Bossart, Nathan 2018-10-01 <69FD7E51-2B13-41FD-9438-17395C73F5BF@amazon.com>

1. Do we really need two functions, one without input argument
to list the default tablespace?
I think that anybody who uses with such a function whould
be able to feed the OID of "pg_default".

That seems reasonable to me. I've removed the second version of the
function.

You could make that one argument have a DEFAULT value that makes it
act on pg_default.

Christoph

#7Laurenz Albe
laurenz.albe@cybertec.at
In reply to: Christoph Berg (#6)
Re: pg_ls_tmpdir()

Christoph Berg wrote:

Re: Bossart, Nathan 2018-10-01 <69FD7E51-2B13-41FD-9438-17395C73F5BF@amazon.com>

1. Do we really need two functions, one without input argument
to list the default tablespace?
I think that anybody who uses with such a function whould
be able to feed the OID of "pg_default".

That seems reasonable to me. I've removed the second version of the
function.

You could make that one argument have a DEFAULT value that makes it
act on pg_default.

I looked at that, and I don't think you can add a DEFAULT for
system functions installed during bootstrap.
At least I failed in the attempt.

Yours,
Laurenz Albe

#8Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Laurenz Albe (#7)
Re: pg_ls_tmpdir()

On 10/02/2018 08:00 AM, Laurenz Albe wrote:

Christoph Berg wrote:

Re: Bossart, Nathan 2018-10-01 <69FD7E51-2B13-41FD-9438-17395C73F5BF@amazon.com>

1. Do we really need two functions, one without input argument
to list the default tablespace?
I think that anybody who uses with such a function whould
be able to feed the OID of "pg_default".

That seems reasonable to me. I've removed the second version of the
function.

You could make that one argument have a DEFAULT value that makes it
act on pg_default.

I looked at that, and I don't think you can add a DEFAULT for
system functions installed during bootstrap.
At least I failed in the attempt.

See the bottom of src/backend/catalog/system_views.sql starting around
line 1010.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#9Bossart, Nathan
bossartn@amazon.com
In reply to: Andrew Dunstan (#8)
Re: pg_ls_tmpdir()

On 10/2/18, 7:22 AM, "Andrew Dunstan" <andrew.dunstan@2ndquadrant.com> wrote:

On 10/02/2018 08:00 AM, Laurenz Albe wrote:

Christoph Berg wrote:

Re: Bossart, Nathan 2018-10-01 <69FD7E51-2B13-41FD-9438-17395C73F5BF@amazon.com>

1. Do we really need two functions, one without input argument
to list the default tablespace?
I think that anybody who uses with such a function whould
be able to feed the OID of "pg_default".

That seems reasonable to me. I've removed the second version of the
function.

You could make that one argument have a DEFAULT value that makes it
act on pg_default.

I looked at that, and I don't think you can add a DEFAULT for
system functions installed during bootstrap.
At least I failed in the attempt.

See the bottom of src/backend/catalog/system_views.sql starting around
line 1010.

AFAICT the cleanest way to do this in system_views.sql is to hard-code
the pg_default tablespace OID in the DEFAULT expression. So, it might
be best to use the two function approach if we want pg_ls_tmpdir() to
default to the pg_default tablespace.

Nathan

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bossart, Nathan (#9)
Re: pg_ls_tmpdir()

"Bossart, Nathan" <bossartn@amazon.com> writes:

On 10/2/18, 7:22 AM, "Andrew Dunstan" <andrew.dunstan@2ndquadrant.com> wrote:

See the bottom of src/backend/catalog/system_views.sql starting around
line 1010.

AFAICT the cleanest way to do this in system_views.sql is to hard-code
the pg_default tablespace OID in the DEFAULT expression. So, it might
be best to use the two function approach if we want pg_ls_tmpdir() to
default to the pg_default tablespace.

That would be pretty bletcherous, so +1 for just making two C functions.

regards, tom lane

#11Bossart, Nathan
bossartn@amazon.com
In reply to: Tom Lane (#10)
1 attachment(s)
Re: pg_ls_tmpdir()

On 10/2/18, 11:46 AM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:

"Bossart, Nathan" <bossartn@amazon.com> writes:

On 10/2/18, 7:22 AM, "Andrew Dunstan" <andrew.dunstan@2ndquadrant.com> wrote:

See the bottom of src/backend/catalog/system_views.sql starting around
line 1010.

AFAICT the cleanest way to do this in system_views.sql is to hard-code
the pg_default tablespace OID in the DEFAULT expression. So, it might
be best to use the two function approach if we want pg_ls_tmpdir() to
default to the pg_default tablespace.

That would be pretty bletcherous, so +1 for just making two C functions.

Alright, here's an updated patch.

Nathan

Attachments:

pg_ls_tmpdir_v3.patchapplication/octet-stream; name=pg_ls_tmpdir_v3.patchDownload
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 9a7f683658..1862ed649d 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -20355,6 +20355,20 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
         role and may be granted to other non-superuser roles.
        </entry>
       </row>
+      <row>
+       <entry>
+        <literal><function>pg_ls_tmpdir(<optional><parameter>tablespace</parameter> <type>oid</type></optional>)</function></literal>
+       </entry>
+       <entry><type>setof record</type></entry>
+       <entry>
+        List the name, size, and last modification time of files in the
+        temporary directory for <parameter>tablespace</parameter>.  If
+        <parameter>tablespace</parameter> is not provided, the
+        <literal>pg_default</literal> tablespace is used.  Access is granted to
+        members of the <literal>pg_monitor</literal> role and may be granted to
+        other non-superuser roles.
+       </entry>
+      </row>
       <row>
        <entry>
         <literal><function>pg_read_file(<parameter>filename</parameter> <type>text</type> [, <parameter>offset</parameter> <type>bigint</type>, <parameter>length</parameter> <type>bigint</type> [, <parameter>missing_ok</parameter> <type>boolean</type>] ])</function></literal>
@@ -20428,6 +20442,19 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
     <command>GRANT</command>.
    </para>
 
+   <indexterm>
+    <primary>pg_ls_tmpdir</primary>
+   </indexterm>
+   <para>
+    <function>pg_ls_tmpdir</function> returns the name, size, and last modified
+    time (mtime) of each file in the temporary file directory for the specified
+    <parameter>tablespace</parameter>.  If <parameter>tablespace</parameter> is
+    not provided, the <literal>pg_default</literal> tablespace is used.  By
+    default only superusers and members of the <literal>pg_monitor</literal>
+    role can use this function.  Access may be granted to others using
+    <command>GRANT</command>.
+   </para>
+
    <indexterm>
     <primary>pg_read_file</primary>
    </indexterm>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 7251552419..020f28cbf6 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1150,6 +1150,8 @@ REVOKE EXECUTE ON FUNCTION lo_export(oid, text) FROM public;
 
 REVOKE EXECUTE ON FUNCTION pg_ls_logdir() FROM public;
 REVOKE EXECUTE ON FUNCTION pg_ls_waldir() FROM public;
+REVOKE EXECUTE ON FUNCTION pg_ls_tmpdir() FROM public;
+REVOKE EXECUTE ON FUNCTION pg_ls_tmpdir(oid) FROM public;
 
 REVOKE EXECUTE ON FUNCTION pg_read_file(text) FROM public;
 REVOKE EXECUTE ON FUNCTION pg_read_file(text,bigint,bigint) FROM public;
@@ -1170,6 +1172,8 @@ REVOKE EXECUTE ON FUNCTION pg_ls_dir(text,boolean,boolean) FROM public;
 --
 GRANT EXECUTE ON FUNCTION pg_ls_logdir() TO pg_monitor;
 GRANT EXECUTE ON FUNCTION pg_ls_waldir() TO pg_monitor;
+GRANT EXECUTE ON FUNCTION pg_ls_tmpdir() TO pg_monitor;
+GRANT EXECUTE ON FUNCTION pg_ls_tmpdir(oid) TO pg_monitor;
 
 GRANT pg_read_all_settings TO pg_monitor;
 GRANT pg_read_all_stats TO pg_monitor;
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index a97cbea248..85bea8d502 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -23,6 +23,7 @@
 #include "access/htup_details.h"
 #include "access/xlog_internal.h"
 #include "catalog/pg_authid.h"
+#include "catalog/pg_tablespace_d.h"
 #include "catalog/pg_type.h"
 #include "funcapi.h"
 #include "mb/pg_wchar.h"
@@ -31,6 +32,7 @@
 #include "storage/fd.h"
 #include "utils/builtins.h"
 #include "utils/memutils.h"
+#include "utils/syscache.h"
 #include "utils/timestamp.h"
 
 typedef struct
@@ -520,7 +522,7 @@ pg_ls_dir_1arg(PG_FUNCTION_ARGS)
 
 /* Generic function to return a directory listing of files */
 static Datum
-pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir)
+pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok)
 {
 	FuncCallContext *funcctx;
 	struct dirent *de;
@@ -549,10 +551,18 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir)
 		fctx->dirdesc = AllocateDir(fctx->location);
 
 		if (!fctx->dirdesc)
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not open directory \"%s\": %m",
-							fctx->location)));
+		{
+			if (missing_ok && errno == ENOENT)
+			{
+				MemoryContextSwitchTo(oldcontext);
+				SRF_RETURN_DONE(funcctx);
+			}
+			else
+				ereport(ERROR,
+						(errcode_for_file_access(),
+						 errmsg("could not open directory \"%s\": %m",
+								fctx->location)));
+		}
 
 		funcctx->user_fctx = fctx;
 		MemoryContextSwitchTo(oldcontext);
@@ -601,12 +611,50 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir)
 Datum
 pg_ls_logdir(PG_FUNCTION_ARGS)
 {
-	return pg_ls_dir_files(fcinfo, Log_directory);
+	return pg_ls_dir_files(fcinfo, Log_directory, false);
 }
 
 /* Function to return the list of files in the WAL directory */
 Datum
 pg_ls_waldir(PG_FUNCTION_ARGS)
 {
-	return pg_ls_dir_files(fcinfo, XLOGDIR);
+	return pg_ls_dir_files(fcinfo, XLOGDIR, false);
+}
+
+/*
+ * Generic function to return the list of files in pgsql_tmp
+ */
+static Datum
+pg_ls_tmpdir(FunctionCallInfo fcinfo, Oid tblspc)
+{
+	char		path[MAXPGPATH];
+
+	if (!SearchSysCacheExists1(TABLESPACEOID, ObjectIdGetDatum(tblspc)))
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_OBJECT),
+				 errmsg("tablespace with OID %u does not exist",
+						tblspc)));
+
+	TempTablespacePath(path, tblspc);
+	return pg_ls_dir_files(fcinfo, path, true);
+}
+
+/*
+ * Function to return the list of temporary files in the pg_default tablespace's
+ * pgsql_tmp directory
+ */
+Datum
+pg_ls_tmpdir_noargs(PG_FUNCTION_ARGS)
+{
+	return pg_ls_tmpdir(fcinfo, DEFAULTTABLESPACE_OID);
+}
+
+/*
+ * Function to return the list of temporary files in the specified tablespace's
+ * pgsql_tmp directory
+ */
+Datum
+pg_ls_tmpdir_1arg(PG_FUNCTION_ARGS)
+{
+	return pg_ls_tmpdir(fcinfo, PG_GETARG_OID(0));
 }
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 8e4145f42b..8579822bcd 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10199,6 +10199,16 @@
   provolatile => 'v', prorettype => 'record', proargtypes => '',
   proallargtypes => '{text,int8,timestamptz}', proargmodes => '{o,o,o}',
   proargnames => '{name,size,modification}', prosrc => 'pg_ls_waldir' },
+{ oid => '5029', descr => 'list files in the pgsql_tmp directory',
+  proname => 'pg_ls_tmpdir', procost => '10', prorows => '20', proretset => 't',
+  provolatile => 'v', prorettype => 'record', proargtypes => '',
+  proallargtypes => '{text,int8,timestamptz}', proargmodes => '{o,o,o}',
+  proargnames => '{name,size,modification}', prosrc => 'pg_ls_tmpdir_noargs' },
+{ oid => '5030', descr => 'list files in the pgsql_tmp directory',
+  proname => 'pg_ls_tmpdir', procost => '10', prorows => '20', proretset => 't',
+  provolatile => 'v', prorettype => 'record', proargtypes => 'oid',
+  proallargtypes => '{oid,text,int8,timestamptz}', proargmodes => '{i,o,o,o}',
+  proargnames => '{tablespace,name,size,modification}', prosrc => 'pg_ls_tmpdir_1arg' },
 
 # hash partitioning constraint function
 { oid => '5028', descr => 'hash partition CHECK constraint',
#12Laurenz Albe
laurenz.albe@cybertec.at
In reply to: Bossart, Nathan (#11)
Re: pg_ls_tmpdir()

Bossart, Nathan wrote:

AFAICT the cleanest way to do this in system_views.sql is to hard-code
the pg_default tablespace OID in the DEFAULT expression. So, it might
be best to use the two function approach if we want pg_ls_tmpdir() to
default to the pg_default tablespace.

That would be pretty bletcherous, so +1 for just making two C functions.

Alright, here's an updated patch.

Looks, good; marking as "ready for committer".

Yours,
Laurenz Albe

#13Michael Paquier
michael@paquier.xyz
In reply to: Laurenz Albe (#12)
Re: pg_ls_tmpdir()

On Thu, Oct 04, 2018 at 02:23:34PM +0200, Laurenz Albe wrote:

Bossart, Nathan wrote:

Alright, here's an updated patch.

Looks, good; marking as "ready for committer".

Like Tom, I think it is less dirty to use the two-function approach.
Committed this way with a catalog version bump.
--
Michael

#14Bossart, Nathan
bossartn@amazon.com
In reply to: Michael Paquier (#13)
Re: pg_ls_tmpdir()

On 10/4/18, 7:31 PM, "Michael Paquier" <michael@paquier.xyz> wrote:

Committed this way with a catalog version bump.

Thanks!

Nathan