Re: [PATCH v1] pg_ls_tmpdir to show directories - Mailing list pgsql-hackers

From Justin Pryzby
Subject Re: [PATCH v1] pg_ls_tmpdir to show directories
Date
Msg-id 20191228101650.GG12890@telsasoft.com
Whole thread Raw
In response to Re: [PATCH v1] pg_ls_tmpdir to show directories  (Fabien COELHO <coelho@cri.ensmp.fr>)
Responses Re: [PATCH v1] pg_ls_tmpdir to show directories
List pgsql-hackers
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)));


Attachment

pgsql-hackers by date:

Previous
From: Fabien COELHO
Date:
Subject: Re: [PATCH v1] pg_ls_tmpdir to show directories
Next
From: Julien Rouhaud
Date:
Subject: Re: Implementing Incremental View Maintenance