Re: pg_ls_tmpdir()

Bossart, Nathan wrote:
> Attached is a patch to add a pg_ls_tmpdir() function that lists the
> contents of a specified tablespace's pgsql_tmp directory. This is
> very similar to the existing pg_ls_logdir() and pg_ls_waldir()
> functions.
>
> By providing more visibility into the temporary file directories,
> users can more easily track queries that are consuming a lot of disk
> space for temporary files. This function already provides enough
> information to calculate the total temporary file usage per PID, but
> it might be worthwhile to create a system view that does this, too.
>
> I am submitting this patch for consideration in commitfest 2018-11.

The patch applies, builds without warning and passes "make check-world".

Since pgsql_tmp is only created when the first temp file is written,
calling the function on a newly initdb'ed data directory fails with

ERROR: could not open directory "base/pgsql_tmp": No such file or directory

This should be fixed; it shouldn't be an error.

Calling the function with a non-existing tablespace OID produces:

ERROR: could not open directory "pg_tblspc/1665/PG_12_201809121/pgsql_tmp": No such file or directory

Wouldn't it be better to have a check if the tablespace exists?

About the code:
The catversion bump shouldn't be part of the patch, it will
rot too quickly. The committer is supposed to add it.

I think the idea to have such a function is fine, but I have two doubts:

1. Do we really need two functions, one without input argument
to list the default tablespace?
I think that anybody who uses with such a function whould
be able to feed the OID of "pg_default".

2. There already are functions "pg_ls_logdir" and "pg_ls_waldir",
and I can imagine new requests, e.g. pg_ls_datafiles() to list the
data files in a database directory.

It may make sense to have a generic function like

pg_ls_dir(dirname text, tablespace oid)

instead. But maybe that's taking it too far...

I'll set the patch to "waiting for author".

Yours,
Laurenz Albe