public inbox for [email protected]
help / color / mirror / Atom feedFrom: Justin Pryzby <[email protected]>
To: Fabien COELHO <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: David Steele <[email protected]>
Cc: [email protected]
Cc: Bossart, Nathan <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: Tom Lane <[email protected]>
Subject: Re: pg_ls_tmpdir to show directories and shared filesets (and pg_ls_*)
Date: Tue, 17 Mar 2020 14:04:01 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <alpine.DEB.2.21.2003170828250.17584@pseudo>
References: <[email protected]>
<[email protected]>
<[email protected]>
<alpine.DEB.2.21.2003170828250.17584@pseudo>
On Tue, Mar 17, 2020 at 10:21:48AM +0100, Fabien COELHO wrote:
>
> About v13, seens as one patch:
>
> Function "pg_ls_dir_metadata" documentation suggests a variable number of
> arguments with brackets, but parameters are really mandatory.
Fixed, and added tests on 1 and 3 arg versions of both pg_ls_dir() and
pg_ls_dir_metadata().
It seems like the only way to make variable number of arguments is is with
multiple entries in pg_proc.dat, one for each "number of" arguments. Is that
right ?
> The example in the documentation could be better indented. Also, ISTM that
> there are two implicit laterals (format & pg_ls_dir_recurse) that I would
> make explicit. I'd use the pcs alias explicitely. I'd use meaningful aliases
> (eg ts instead of b, …).
> On reflection, I think that a boolean "isdir" column is a bad idea because
> it is not extensible. I'd propose to switch to the standard "ls" approach of
> providing the type as one character: '-' for regular, 'd' for directory, 'l'
> for link, 's' for socket, 'c' for character special…
I think that's outside the scope of the patch, since I'd want to change
pg_stat_file; that's where I borrowed "isdir" from, for consistency.
Note that both LS_DIR_HISTORIC and LS_DIR_MODERN include LS_DIR_SKIP_SPECIAL,
so only pg_ls_dir itself show specials, so they way to do it would be to 1)
change pg_stat_file to expose the file's "type", 2) use pg_ls_dir() AS a,
lateral pg_stat_file(a) AS b, 3) then consider also changing LS_DIR_MODERN and
all the existing pg_ls_*.
> ISTM that "lstat" is not available on windows, which suggests to call "stat"
> always, and then "lstat" on un*x and pg ports stuff on win.
I believe that's handled here.
src/include/port/win32_port.h:#define lstat(path, sb) stat(path, sb)
> I'm wondering about the restriction on directories only. Why should it not
> work on a file? Can it be easily extended to work on a simple file? If so,
> it could be just "pg_ls".
I think that's a good idea, except it doesn't fit with what the code does:
AllocDir() and ReadDir(). Instead, use pg_stat_file() for that.
Hm, I realized that the existing pg_ls_dir_metadata was skipping links to dirs,
since !ISREG(). So changed to use both stat() and lstat().
--
Justin
view thread (60+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: pg_ls_tmpdir to show directories and shared filesets (and pg_ls_*)
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox