From: | Justin Pryzby <pryzby(at)telsasoft(dot)com> |
---|---|
To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
Cc: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, David Steele <david(at)pgmasters(dot)net>, pgsql-hackers(at)postgresql(dot)org, "Bossart, Nathan" <bossartn(at)amazon(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Subject: | Re: pg_ls_tmpdir to show directories and shared filesets (and pg_ls_*) |
Date: | 2020-03-13 13:12:32 |
Message-ID: | 20200313131232.GO29065@telsasoft.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
@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
Attachment | Content-Type | Size |
---|---|---|
v10-0001-Document-historic-behavior-about-hiding-director.patch | text/x-diff | 1.7 KB |
v10-0002-Document-historic-behavior-about-hiding-director.patch | text/x-diff | 906 bytes |
v10-0003-Add-tests-exercizing-pg_ls_tmpdir.patch | text/x-diff | 4.3 KB |
v10-0004-Add-pg_ls_dir_metadata-to-list-a-dir-with-file-m.patch | text/x-diff | 9.9 KB |
v10-0005-pg_ls_tmpdir-to-show-isdir-argument.patch | text/x-diff | 7.6 KB |
v10-0006-pg_ls_-dir-to-show-directories-and-isdir-column.patch | text/x-diff | 12.8 KB |
v10-0007-Add-pg_ls_dir_recurse-to-show-dir-recursively.patch | text/x-diff | 2.7 KB |
v10-0008-pg_ls_logdir-to-ignore-error-if-initial-top-dir-.patch | text/x-diff | 3.0 KB |
v10-0009-pg_ls_-dir-to-return-all-the-metadata-from-pg_st.patch | text/x-diff | 15.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Victor Wagner | 2020-03-13 13:16:10 | Re: make check crashes on POWER8 machine |
Previous Message | Justin Pryzby | 2020-03-13 12:43:59 | Re: make check crashes on POWER8 machine |