On Sat, Dec 28, 2019 at 07:52:55AM +0100, Fabien COELHO wrote:
> >>Why not simply showing the files underneath their directories?
> >>
> >>  /path/to/tmp/file1
> >>  /path/to/tmp/subdir1/file2
> >>
> >>In which case probably showing the directory itself is not useful,
> >>and the is_dir column could be dropped?
> >
> >The names are expected to look like this:
> >
> >$ sudo find /var/lib/pgsql/12/data/base/pgsql_tmp -ls
> >142977    4 drwxr-x---   3 postgres postgres     4096 Dec 27 13:51 
> >/var/lib/pgsql/12/data/base/pgsql_tmp
> >169868    4 drwxr-x---   2 postgres postgres     4096 Dec  7 01:35 
> >/var/lib/pgsql/12/data/base/pgsql_tmp/pgsql_tmp11025.0.sharedfileset
> >169347 5492 -rw-r-----   1 postgres postgres  5619712 Dec  7 01:35 
> >/var/lib/pgsql/12/data/base/pgsql_tmp/pgsql_tmp11025.0.sharedfileset/0.0
> >169346 5380 -rw-r-----   1 postgres postgres  5505024 Dec  7 01:35 
> >/var/lib/pgsql/12/data/base/pgsql_tmp/pgsql_tmp11025.0.sharedfileset/1.0
> >
> >I think we'd have to show sudbdir/file1, subdir/file2, not just file1, file2.
> >It doesn't seem useful or nice to show a bunch of files called 0.0 or 1.0.
> >Actually the results should be unique, either on filename or (dir,file).
> 
> Ok, so this suggests recursing into subdirs, which requires to make a
> separate function of the inner loop.

Yea, it suggests that; but, SRF_RETURN_NEXT doesn't make that very easy.
It'd need to accept the fcinfo argument, and pg_ls_dir_files would call it once
for every tuple to be returned.  So it's recursive and saves its state...

The attached is pretty ugly, but I can't see how to do better.
The alternative seems to be to build up a full list of pathnames in the SRF
initial branch, and stat them all later.  Or stat them all in the INITIAL case,
and keep a list of stat structures to be emited during future calls.

BTW, it seems to me this error message should be changed:

                snprintf(path, sizeof(path), "%s/%s", fctx->location, 
de->d_name);
                if (stat(path, &attrib) < 0)
                        ereport(ERROR,
                                        (errcode_for_file_access(),
-                                        errmsg("could not stat directory 
\"%s\": %m", dir)));
+                                        errmsg("could not stat file \"%s\": 
%m", path)));

>From fd88be5f1687354d9990fb1838adc0db36bc6dde Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Fri, 27 Dec 2019 23:34:14 -0600
Subject: [PATCH v2 1/2] BUG: in errmsg

---
 src/backend/utils/adt/genfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 5d4f26a..c978e15 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -590,7 +590,7 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok)
 		if (stat(path, &attrib) < 0)
 			ereport(ERROR,
 					(errcode_for_file_access(),
-					 errmsg("could not stat directory \"%s\": %m", dir)));
+					 errmsg("could not stat file \"%s\": %m", path)));
 
 		/* Ignore anything but regular files */
 		if (!S_ISREG(attrib.st_mode))
-- 
2.7.4

>From fff91aec87f635755527e91aebb7554fa6385fec Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sat, 14 Dec 2019 16:22:15 -0600
Subject: [PATCH v2 2/2] pg_ls_tmpdir to show directories

See also 9cd92d1a33699f86aa53d44ab04cc3eb50c18d11
---
 doc/src/sgml/func.sgml           |  14 +++--
 src/backend/utils/adt/genfile.c  | 132 ++++++++++++++++++++++++++-------------
 src/include/catalog/catversion.h |   2 +-
 3 files changed, 96 insertions(+), 52 deletions(-)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 5a98158..8abc643 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -21922,12 +21922,14 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
        </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.
+        For files in the temporary directory for
+        <parameter>tablespace</parameter>, list the name, size, and last modification time.
+        Files beneath a first-level directory are shown, and include a pathname
+        component of their parent directory; such files are used by parallel processes.
+        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>
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index c978e15..2b540e9 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -37,8 +37,12 @@
 
 typedef struct
 {
-	char	   *location;
-	DIR		   *dirdesc;
+	/*
+	 * These arrays allow recursing a single time to handle subdirs.
+	 * When not recursing, location[1] = dirdesc[1] = NULL;
+	 */
+	char	   *location[2];
+	DIR		   *dirdesc[2];
 	bool		include_dot_dirs;
 } directory_fctx;
 
@@ -469,12 +473,13 @@ pg_ls_dir(PG_FUNCTION_ARGS)
 		oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
 
 		fctx = palloc(sizeof(directory_fctx));
-		fctx->location = convert_and_check_filename(PG_GETARG_TEXT_PP(0));
+		memset(fctx, 0, sizeof(*fctx));
+		fctx->location[0] = convert_and_check_filename(PG_GETARG_TEXT_PP(0));
 
 		fctx->include_dot_dirs = include_dot_dirs;
-		fctx->dirdesc = AllocateDir(fctx->location);
+		fctx->dirdesc[0] = AllocateDir(fctx->location[0]);
 
-		if (!fctx->dirdesc)
+		if (!fctx->dirdesc[0])
 		{
 			if (missing_ok && errno == ENOENT)
 			{
@@ -485,7 +490,7 @@ pg_ls_dir(PG_FUNCTION_ARGS)
 				ereport(ERROR,
 						(errcode_for_file_access(),
 						 errmsg("could not open directory \"%s\": %m",
-								fctx->location)));
+								fctx->location[0])));
 		}
 		funcctx->user_fctx = fctx;
 		MemoryContextSwitchTo(oldcontext);
@@ -494,7 +499,7 @@ pg_ls_dir(PG_FUNCTION_ARGS)
 	funcctx = SRF_PERCALL_SETUP();
 	fctx = (directory_fctx *) funcctx->user_fctx;
 
-	while ((de = ReadDir(fctx->dirdesc, fctx->location)) != NULL)
+	while ((de = ReadDir(fctx->dirdesc[0], fctx->location[0])) != NULL)
 	{
 		if (!fctx->include_dot_dirs &&
 			(strcmp(de->d_name, ".") == 0 ||
@@ -504,7 +509,7 @@ pg_ls_dir(PG_FUNCTION_ARGS)
 		SRF_RETURN_NEXT(funcctx, CStringGetTextDatum(de->d_name));
 	}
 
-	FreeDir(fctx->dirdesc);
+	FreeDir(fctx->dirdesc[0]);
 
 	SRF_RETURN_DONE(funcctx);
 }
@@ -522,12 +527,12 @@ pg_ls_dir_1arg(PG_FUNCTION_ARGS)
 	return pg_ls_dir(fcinfo);
 }
 
-/* Generic function to return a directory listing of files */
+/* Generic function to return a directory listing of files (and optionally dirs) */
+/* Calls itself recursively to handle dirs, if requested */
 static Datum
-pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok)
+pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok, bool dir_ok)
 {
 	FuncCallContext *funcctx;
-	struct dirent *de;
 	directory_fctx *fctx;
 
 	if (SRF_IS_FIRSTCALL())
@@ -539,6 +544,7 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok)
 		oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
 
 		fctx = palloc(sizeof(directory_fctx));
+		memset(fctx, 0, sizeof(*fctx));
 
 		tupdesc = CreateTemplateTupleDesc(3);
 		TupleDescInitEntry(tupdesc, (AttrNumber) 1, "name",
@@ -547,10 +553,11 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok)
 						   INT8OID, -1, 0);
 		TupleDescInitEntry(tupdesc, (AttrNumber) 3, "modification",
 						   TIMESTAMPTZOID, -1, 0);
+
 		funcctx->tuple_desc = BlessTupleDesc(tupdesc);
 
-		fctx->location = pstrdup(dir);
-		fctx->dirdesc = AllocateDir(fctx->location);
+		fctx->location[0] = pstrdup(dir);
+		fctx->dirdesc[0] = AllocateDir(fctx->location[0]);
 
 		if (!fctx->dirdesc)
 		{
@@ -563,7 +570,7 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok)
 				ereport(ERROR,
 						(errcode_for_file_access(),
 						 errmsg("could not open directory \"%s\": %m",
-								fctx->location)));
+								fctx->location[1])));
 		}
 
 		funcctx->user_fctx = fctx;
@@ -573,39 +580,73 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok)
 	funcctx = SRF_PERCALL_SETUP();
 	fctx = (directory_fctx *) funcctx->user_fctx;
 
-	while ((de = ReadDir(fctx->dirdesc, fctx->location)) != NULL)
-	{
-		Datum		values[3];
-		bool		nulls[3];
-		char		path[MAXPGPATH * 2];
-		struct stat attrib;
-		HeapTuple	tuple;
-
-		/* Skip hidden files */
-		if (de->d_name[0] == '.')
-			continue;
+	while (1) {
+		struct dirent *de;
+		char *location;
+		DIR *dirdesc;
 
-		/* Get the file info */
-		snprintf(path, sizeof(path), "%s/%s", fctx->location, de->d_name);
-		if (stat(path, &attrib) < 0)
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not stat file \"%s\": %m", path)));
+		location = fctx->location[1] ? fctx->location[1] : fctx->location[0];
+		dirdesc = fctx->dirdesc[1] ? fctx->dirdesc[1] : fctx->dirdesc[0];
 
-		/* Ignore anything but regular files */
-		if (!S_ISREG(attrib.st_mode))
-			continue;
+		while ((de = ReadDir(dirdesc, location)) != NULL)
+		{
+			char		path[MAXPGPATH * 2];
+			Datum		values[3];
+			bool		nulls[3];
+			HeapTuple	tuple;
+
+			struct stat	attrib;
+			/* Skip hidden files */
+			if (de->d_name[0] == '.')
+				continue;
+
+			/* Get the file info */
+			snprintf(path, sizeof(path), "%s/%s", location, de->d_name);
+			if (stat(path, &attrib) < 0)
+				ereport(ERROR,
+						(errcode_for_file_access(),
+						 errmsg("could not stat file \"%s\": %m", path)));
+
+			/* Ignore anything but regular files or (if requested) dirs */
+			if (S_ISDIR(attrib.st_mode)) {
+				MemoryContext oldcontext;
+
+				/* Note: decend into dirs, but do not return a tuple for the dir itself */
+				/* Do not expect dirs more than one level deep */
+				// Maybe do something other than ignore, like show the dir itself instead of recursing ?
+				if (!dir_ok || fctx->location[1])
+					continue;
+
+				oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+				fctx->location[1] = pstrdup(path);
+				fctx->dirdesc[1] = AllocateDir(path);
+				MemoryContextSwitchTo(oldcontext);
+				return pg_ls_dir_files(fcinfo, path, missing_ok, false);
+			} else if (!S_ISREG(attrib.st_mode))
+				continue;
+
+			if (fctx->location[1])
+				/* We've already catted together the paths before recursing, so find the last component */
+				values[0] = CStringGetTextDatum(path+1+strlen(fctx->location[0]));
+			else
+				values[0] = CStringGetTextDatum(de->d_name);
+			values[1] = Int64GetDatum((int64) attrib.st_size);
+			values[2] = TimestampTzGetDatum(time_t_to_timestamptz(attrib.st_mtime));
+
+			memset(nulls, 0, sizeof(nulls));
+			tuple = heap_form_tuple(funcctx->tuple_desc, values, nulls);
+			SRF_RETURN_NEXT(funcctx, HeapTupleGetDatum(tuple));
+		}
 
-		values[0] = CStringGetTextDatum(de->d_name);
-		values[1] = Int64GetDatum((int64) attrib.st_size);
-		values[2] = TimestampTzGetDatum(time_t_to_timestamptz(attrib.st_mtime));
-		memset(nulls, 0, sizeof(nulls));
+		if (!fctx->dirdesc[1])
+			break;
 
-		tuple = heap_form_tuple(funcctx->tuple_desc, values, nulls);
-		SRF_RETURN_NEXT(funcctx, HeapTupleGetDatum(tuple));
+		FreeDir(fctx->dirdesc[1]);
+		fctx->location[1] = NULL;
+		fctx->dirdesc[1] = NULL;
 	}
 
-	FreeDir(fctx->dirdesc);
+	FreeDir(fctx->dirdesc[0]);
 	SRF_RETURN_DONE(funcctx);
 }
 
@@ -613,18 +654,19 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok)
 Datum
 pg_ls_logdir(PG_FUNCTION_ARGS)
 {
-	return pg_ls_dir_files(fcinfo, Log_directory, false);
+	return pg_ls_dir_files(fcinfo, Log_directory, false, 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, false);
+	return pg_ls_dir_files(fcinfo, XLOGDIR, false, false);
 }
 
 /*
  * Generic function to return the list of files in pgsql_tmp
+ * Files are also shown one level deep, with their subdir prefix.
  */
 static Datum
 pg_ls_tmpdir(FunctionCallInfo fcinfo, Oid tblspc)
@@ -638,7 +680,7 @@ pg_ls_tmpdir(FunctionCallInfo fcinfo, Oid tblspc)
 						tblspc)));
 
 	TempTablespacePath(path, tblspc);
-	return pg_ls_dir_files(fcinfo, path, true);
+	return pg_ls_dir_files(fcinfo, path, true, true);
 }
 
 /*
@@ -667,5 +709,5 @@ pg_ls_tmpdir_1arg(PG_FUNCTION_ARGS)
 Datum
 pg_ls_archive_statusdir(PG_FUNCTION_ARGS)
 {
-	return pg_ls_dir_files(fcinfo, XLOGDIR "/archive_status", true);
+	return pg_ls_dir_files(fcinfo, XLOGDIR "/archive_status", true, false);
 }
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index eca67a1..e2c05be 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
  */
 
 /*							yyyymmddN */
-#define CATALOG_VERSION_NO	201911242
+#define CATALOG_VERSION_NO	201911243
 
 #endif
-- 
2.7.4

Reply via email to