Re: pg_ls_tmpdir to show directories and shared filesets

On Sat, Mar 07, 2020 at 03:14:37PM +0100, Fabien COELHO wrote:
> Some feedback about the v7 patch set.

Thanks for looking again

> About v7.1, seems ok.
> 
> About v7.2 & v7.3 seems ok, altought the two could be merged.

These are separate since I proprose that one should be backpatched to v12 and
the other to v10.

> About v7.4:
...
> It seems that lists are used as FIFO structures by appending, fetching &
> deleting last, all of which are O(n). ISTM it would be better to use the
> head of the list by inserting, getting and deleting first, which are O(1).

I think you're referring to linked lists, but pglists are now arrays, for which
that's backwards.  See 1cff1b95a and d97b714a2.  For example, list_delete_last
says:
 * This is the opposite of list_delete_first(), but is noticeably cheaper
 * with a long list, since no data need be moved.

> ISTM that several instances of: "pg_ls_dir_files(..., true, false);" should
> be "pg_ls_dir_files(..., true, DIR_HIDE);".

Oops, that affects an intermediate commit and maybe due to merge conflict.
Thanks.

> About v7.5 looks like a doc update which should be merged with v7.4.

No, v7.5 updates pg_proc.dat and changes the return type of two functions.
It's a short commit since all the infrastructure is implemented to make the
functions do whatever we want.  But it's deliberately separate since I'm
proposing a breaking change, and one that hasn't been discussed until now.

> Alas, ISTM that there are no tests on any of these functions:-(

Yeah.  Everything that includes any output is going to include timestamps;
those could be filtered out.  waldir is going to have random filenames, and a
differing number of rows.  But we should exercize pg_ls_dir_files at least
once..

My previous version had a bug with ignore_missing with pg_ls_tmpdir, which
would've been caught by a test like:
SELECT FROM pg_ls_tmpdir() WHERE name='Does not exist'; -- Never true, so the function runs to completion but returns
zerorows.
 

The 0006 commit changes that for logdir, too.  Without 0006, that will ERROR if
the dir doesn't exist (which I think would be the default during regression
tests).

It'd be nice to run pg_ls_tmpdir before the tmpdir exists, and again
afterwards.  But I'm having trouble finding a single place to put it.  The
closest I can find is dbsize.sql.  Any ideas ?

-- 
Justin