Re: pg_ls_tmpdir to show directories and shared filesets (and pg_ls_*)

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-06-22 01:53:25
Message-ID: 20200622015325.GG17995@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Jun 07, 2020 at 10:07:19AM +0200, Fabien COELHO wrote:
> Hello Justin,
> > Rebased onto 7b48f1b490978a8abca61e9a9380f8de2a56f266 and renumbered OIDs.

Rebased again on whatever broke func.sgml.

> pg_stat_file() and pg_stat_dir_files() now return a char type, as well as
> the function which call them, but the documentation does not seem to say
> that it is the case.

Fixed, thanks

> I must admit that I'm not a fan on the argument management of
> pg_ls_dir_metadata and pg_ls_dir_metadata_1arg and others. I understand that
> it saves a few lines though, so maybe let it be.

I think you're saying that you don't like the _1arg functions, but they're
needed to allow the regression tests to pass:

| * note: this wrapper is necessary to pass the sanity check in opr_sanity,
| * which checks that all built-in functions that share the implementing C
| * function take the same number of arguments

> There is a comment in pg_ls_dir_files which talks about pg_ls_dir.
>
> Could pg_ls_*dir functions C implementations be dropped in favor of a pure
> SQL implementation, like you did with recurse?

I'm still waiting to hear feedback from a commiter if this is a good idea to
put this into the system catalog. Right now, ts_debug is the only nontrivial
function.

> If so, ISTM that pg_ls_dir_files() could be significantly simplified by
> moving its filtering flag to SQL conditions on "type" and others. That could
> allow not to change the existing function output a keep the "isdir" column
> defined as "type = 'd'" where it was used previously, if someone complains,
> but still have the full capability of "ls". That would also allow to drop
> the "*_1arg" hacks. Basically I'm advocating having 1 or 2 actual C
> functions, and all other variants managed at the SQL level.

You want to get rid of the 1arg stuff and just have one function.

I see your point, but I guess the C function would still need to accept a
"missing_ok" argument, so we need two functions, so there's not much utility in
getting rid of the "include_dot_dirs" argument, which is there for consistency
with pg_ls_dir.

Conceivably we could 1) get rid of pg_ls_dir, and 2) get rid of the
include_dot_dirs argument and 3) maybe make "missing_ok" a required argument;
and, 4) get rid of the C wrapper functions, and replace with a bunch of stuff
like this:

SELECT name, size, access, modification, change, creation, type='d' AS isdir
FROM pg_ls_dir_metadata('pg_wal') WHERE substring(name,1,1)!='.' AND type!='d';

Where the defaults I changed in this patchset still remain to be discussed:
with or without metadata, hidden files, dotdirs.

As I'm still waiting for committer feedback on the first 10 patches, so not
intending to add more.

--
Justin

Attachment Content-Type Size
v19-0001-Document-historic-behavior-of-links-to-directori.patch text/x-diff 1.1 KB
v19-0002-pg_stat_file-and-pg_ls_dir_-to-use-lstat.patch text/x-diff 2.3 KB
v19-0003-Add-tests-on-pg_ls_dir-before-changing-it.patch text/x-diff 2.0 KB
v19-0004-Add-pg_ls_dir_metadata-to-list-a-dir-with-file-m.patch text/x-diff 19.0 KB
v19-0005-pg_ls_tmpdir-to-show-directories-and-isdir-argum.patch text/x-diff 6.7 KB
v19-0006-pg_ls_-dir-to-show-directories-and-isdir-column.patch text/x-diff 8.8 KB
v19-0007-Add-pg_ls_dir_recurse-to-show-dir-recursively.patch text/x-diff 5.8 KB
v19-0008-pg_ls_logdir-to-ignore-error-if-initial-top-dir-.patch text/x-diff 3.1 KB
v19-0009-pg_ls_-dir-to-return-all-the-metadata-from-pg_st.patch text/x-diff 19.5 KB
v19-0010-pg_ls_-to-show-file-type-and-show-special-files.patch text/x-diff 20.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2020-06-22 02:48:40 Re: [PATCH] Initial progress reporting for COPY command
Previous Message Tom Lane 2020-06-22 01:39:37 Re: Improve planner cost estimations for alternative subplans