Re: [PATCH v1] pg_ls_tmpdir to show directories

Поиск
Список
Период
Сортировка
От Justin Pryzby
Тема Re: [PATCH v1] pg_ls_tmpdir to show directories
Дата
Msg-id 20191227170220.GE12890@telsasoft.com
обсуждение исходный текст
Ответы Re: [PATCH v1] pg_ls_tmpdir to show directories  (Fabien COELHO <coelho@cri.ensmp.fr>)
Список pgsql-hackers
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



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Tom Lane
Дата:
Сообщение: Re: use CLZ instruction in AllocSetFreeIndex()
Следующее
От: Teodor Sigaev
Дата:
Сообщение: aggregate crash