public inbox for [email protected]  

help / color / mirror / Atom feed

From: Fabien COELHO <[email protected]>
To: Justin Pryzby <[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: Sun, 7 Jun 2020 10:07:19 +0200 (CEST)
Message-ID: <alpine.DEB.2.22.394.2006070908120.32228@pseudo> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<alpine.DEB.2.21.2003170828250.17584@pseudo>
	<[email protected]>
	<[email protected]>
	<alpine.DEB.2.21.2004121145190.5257@pseudo>
	<[email protected]>
	<[email protected]>


Hello Justin,

> Rebased onto 7b48f1b490978a8abca61e9a9380f8de2a56f266 and renumbered OIDs.

Some feedback about v18, seen as one patch.

Patch applies cleanly & compiles. "make check" is okay.

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.

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.

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?

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.

-- 
Fabien.

view thread (57+ 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: <alpine.DEB.2.22.394.2006070908120.32228@pseudo>

* 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