Date: Fri, 13 Mar 2020 08:12:32 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Fabien COELHO <coelho@cri.ensmp.fr>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>,
	David Steele <david@pgmasters.net>, pgsql-hackers@postgresql.org,
	"Bossart, Nathan" <bossartn@amazon.com>,
	Thomas Munro <thomas.munro@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>
Subject: Re: pg_ls_tmpdir to show directories and shared filesets (and
 pg_ls_*)
Message-ID: <20200313131232.GO29065@telsasoft.com>
References: <20200303200117.GI684@telsasoft.com>
 <20200303202313.GA28076@alvherre.pgsql>
 <20200305161838.GJ684@telsasoft.com>
 <20200306233507.GN684@telsasoft.com>
 <alpine.DEB.2.21.2003071125340.21542@pseudo>
 <20200307214010.GB1357@telsasoft.com>
 <20200310183037.GA29065@telsasoft.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="rCwQ2Y43eQY6RBgR"
Content-Disposition: inline
In-Reply-To: <20200310183037.GA29065@telsasoft.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk


--rCwQ2Y43eQY6RBgR
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

@cfbot: rebased onto 085b6b6679e73b9b386f209b4d625c7bc60597c0

The merge conflict presents another opportunity to solicit comments on the new
approach.  Rather than making "recurse into tmpdir" the end goal:

  - add a function to show metadata of an arbitrary dir;
  - add isdir arguments to pg_ls_* functions (including pg_ls_tmpdir but not
    pg_ls_dir).
  - maybe add pg_ls_dir_recurse, which satisfies the original need;
  - retire pg_ls_dir (does this work with tuplestore?)
  - profit

The alternative seems to be to go back to Alvaro's earlier proposal:
 - not only add "isdir", but also recurse;

I think I would insist on adding a general function to recurse into any dir.
And *optionally* change ps_ls_* to recurse (either by accepting an argument, or
by making that a separate patch to debate).  tuplestore is certainly better
than keeping a stack/List of DIRs for this.

On Tue, Mar 10, 2020 at 01:30:37PM -0500, Justin Pryzby wrote:
> I took a step back, and I wondered whether we should add a generic function for
> listing a dir with metadata, possibly instead of changing the existing
> functions.  Then one could do pg_ls_dir_metadata('pg_wal',false,false);
> 
> Since pg8.1, we have pg_ls_dir() to show a list of files.  Since pg10, we've
> had pg_ls_logdir and pg_ls_waldir, which show not only file names but also
> (some) metadata (size, mtime).  And since pg12, we've had pg_ls_tmpfile and
> pg_ls_archive_statusdir, which also show metadata.
> 
> ...but there's no a function which lists the metadata of an directory other
> than tmp, wal, log.
> 
> One can do this:
> |SELECT b.*, c.* FROM (SELECT 'base' a)a, LATERAL (SELECT a||'/'||pg_ls_dir(a.a)b)b, pg_stat_file(b)c;
> ..but that's not as helpful as allowing:
> |SELECT * FROM pg_ls_dir_metadata('.',true,true);
> 
> There's also no function which recurses into an arbitrary directory, so it
> seems shortsighted to provide a function to recursively list a tmpdir.
> 
> Also, since pg_ls_dir_metadata indicates whether the path is a dir, one can
> write a SQL function to show the dir recursively.  It'd be trivial to plug in
> wal/log/tmp (it seems like tmpdirs of other tablespace's are not entirely
> trivial).
> |SELECT * FROM pg_ls_dir_recurse('base/pgsql_tmp');
> 
> Also, on a neighboring thread[1], Tom indicated that the pg_ls_* functions
> should enumerate all files during the initial call, which sounds like a bad
> idea when recursively showing directories.  If we add a function recursing into
> a directory, we'd need to discuss all the flags to expose to it, like recurse,
> ignore_errors, one_filesystem?, show_dotfiles (and eventually bikeshed all the
> rest of the flags in find(1)).
> 
> My initial patch [2] changed ls_tmpdir to show metadata columns including
> is_dir, but not decend.  It's pretty unfortunate if a function called
> pg_ls_tmpdir hides shared filesets, so maybe it really is best to change that
> (it's new in v12).
> 
> I'm interested to in feedback on the alternative approach, as attached.  The
> final patch to include all the rest of columns shown by pg_stat_file() is more
> of an idea/proposal and not sure if it'll be desirable.  But pg_ls_tmpdir() is
> essentially the same as my v1 patch.
> 
> This is intended to be mostly independent of any fix to the WARNING I reported
> [1].  Since my patch collapses pg_ls_dir into pg_ls_dir_files, we'd only need
> to fix one place.  I'm planning to eventually look into Tom's suggestion of
> returning tuplestore to fix that, and maybe rebase this patchset on top of
> that.
> 
> -- 
> Justin
> 
> [1] https://www.postgresql.org/message-id/flat/20200308173103.GC1357%40telsasoft.com
> [2] https://www.postgresql.org/message-id/20191214224735.GA28433%40telsasoft.com

--rCwQ2Y43eQY6RBgR
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v10-0001-Document-historic-behavior-about-hiding-director.patch"