Re: [PATCH v1] pg_ls_tmpdir to show directories

Re-added -hackers.

Thanks for reviewing.

On Fri, Dec 27, 2019 at 05:22:47PM +0100, Fabien COELHO wrote:
> The implementation simply extends an existing functions with a boolean to
> allow for sub-directories. However, the function does not seem to show
> subdir contents recursively. Should it be the case?

> STM that "//"-comments are not project policy.

Sure, but the patch is less important than the design, which needs to be
addressed first. The goal is to somehow show tmpfiles (or at least dirs) used
by parallel workers. I mentioned a few possible ways, of which this was the
simplest to implement. Showing files beneath the dir is probably good, but
need to decide how to present it. Should there be a column for the dir (null
if not a shared filesets)? Or some other presentation, like a boolean column
"is_shared_fileset".

> I'm unconvinced by the skipping condition:
>
> + if (!S_ISREG(attrib.st_mode) &&
> + (!dir_ok && S_ISDIR(attrib.st_mode)))
> continue;
>
> which is pretty hard to read. ISTM you meant "not A and not (B and C)"
> instead?

I can write it as two ifs. And, it's probably better to say:
if (!ISDIR() || !dir_ok)

..which is same as: !(B && C), as you said.

> Catalog update should be a date + number? Maybe this is best left to the
> committer?

Yes, but I preferred to include it, causing a deliberate conflict, to ensure
it's not forgotten.

Thanks,
Justin