Re: [PATCH v1] pg_ls_tmpdir to show directories
От | Fabien COELHO |
---|---|
Тема | Re: [PATCH v1] pg_ls_tmpdir to show directories |
Дата | |
Msg-id | alpine.DEB.2.21.1912271845000.27864@pseudo обсуждение исходный текст |
Ответ на | Re: [PATCH v1] pg_ls_tmpdir to show directories (Justin Pryzby <pryzby@telsasoft.com>) |
Ответы |
Re: [PATCH v1] pg_ls_tmpdir to show directories
|
Список | pgsql-hackers |
> Re-added -hackers. Indeed, I left it out by accident. I tried to bounce the original mail but it did not work. > 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". Why not simply showing the files underneath their directories? /path/to/tmp/file1 /path/to/tmp/subdir1/file2 In which case probably showing the directory itself is not useful, and the is_dir column could be dropped? >> 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. Hmmm. Not sure it would help that much. At least the condition must be right. Also, the comment should be updated. > And, it's probably better to say: > if (!ISDIR() || !dir_ok) I cannot say I like it. dir_ok is cheaper to test so could come first. > ..which is same as: !(B && C), as you said. Ok, so you confirm that the condition was wrong. >> 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. Ok. -- Fabien.
В списке pgsql-hackers по дате отправления: