Re: [HACKERS] pg_ls_waldir() & pg_ls_logdir()
On Thu, Mar 16, 2017 at 7:05 PM, Robert Haas wrote: > On Thu, Mar 16, 2017 at 6:09 AM, Dave Page wrote: >> Hmm, good point. Google seems to be saying there isn't one. Patch >> updated as you suggest (and I've added back in a function declaration >> that got lost in the rebasing of the last version). > > OK, I took another look at this: > > - The documentation wasn't consistent with itself about the order in > which the three columns were mentioned. I changed it to say name, > size, modification time both places and made the code also return the > columns in that order. And I renamed the columns to name, size, and > modification, the last of which was chosen to match pg_stat_file(). > > - I added an error check for the stat() call. > > - I moved the code to genfile.c where pg_ls_dir() already is; it seems > to fit within the charter of that file. > > - I changed it to build a heap tuple directly instead of converting to > text and then back to datums. Seems less error-prone that way, and > more consistent with what's done elsewhere in genfile.c. > > - I made it use a static-allocated buffer instead of a palloc'd one, > just so it doesn't leak into the surrounding context. > > - I removed the function prototype and instead declared the helper > function static. If there's an intent to expose that function to > extensions, the prototype should be in a header, not the .c file. > > - I adjusted the language in the documentation to be a bit more > similar to what we've done elsewhere. > > With those changes, committed. Thanks! -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_ls_waldir() & pg_ls_logdir()
On Thu, Mar 16, 2017 at 6:09 AM, Dave Page wrote: > Hmm, good point. Google seems to be saying there isn't one. Patch > updated as you suggest (and I've added back in a function declaration > that got lost in the rebasing of the last version). OK, I took another look at this: - The documentation wasn't consistent with itself about the order in which the three columns were mentioned. I changed it to say name, size, modification time both places and made the code also return the columns in that order. And I renamed the columns to name, size, and modification, the last of which was chosen to match pg_stat_file(). - I added an error check for the stat() call. - I moved the code to genfile.c where pg_ls_dir() already is; it seems to fit within the charter of that file. - I changed it to build a heap tuple directly instead of converting to text and then back to datums. Seems less error-prone that way, and more consistent with what's done elsewhere in genfile.c. - I made it use a static-allocated buffer instead of a palloc'd one, just so it doesn't leak into the surrounding context. - I removed the function prototype and instead declared the helper function static. If there's an intent to expose that function to extensions, the prototype should be in a header, not the .c file. - I adjusted the language in the documentation to be a bit more similar to what we've done elsewhere. With those changes, committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_ls_waldir() & pg_ls_logdir()
On Thu, Mar 16, 2017 at 9:54 AM, Thomas Munro wrote: > On Thu, Mar 16, 2017 at 10:40 PM, Dave Page wrote: >>> +const int n = snprintf(NULL, 0, "%lld", attrib.st_size); > > I wonder what the portable printf directive is for off_t. Maybe > better to use INT64_FORMAT and cast to int64? Hmm, good point. Google seems to be saying there isn't one. Patch updated as you suggest (and I've added back in a function declaration that got lost in the rebasing of the last version). -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index a521912317..e15ad77dec 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -19646,7 +19646,8 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); database cluster directory and the log_directory can be accessed. Use a relative path for files in the cluster directory, and a path matching the log_directory configuration setting -for log files. Use of these functions is restricted to superusers. +for log files. Use of these functions is restricted to superusers +except where stated otherwise. @@ -19669,6 +19670,26 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); +pg_ls_logdir() + + setof record + +List the name, last modification time and size of files in the log directory. +Execute permission may be granted to non-superuser roles. + + + + +pg_ls_waldir() + + setof record + +List the name, last modification time and size of files in the WAL directory. +Execute permission may be granted to non-superuser roles. + + + + pg_read_file(filename text [, offset bigint, length bigint [, missing_ok boolean] ]) text @@ -19699,7 +19720,7 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); -All of these functions take an optional missing_ok parameter, +Some of these functions take an optional missing_ok parameter, which specifies the behavior when the file or directory does not exist. If true, the function returns NULL (except pg_ls_dir, which returns an empty result set). If @@ -19720,6 +19741,26 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); +pg_ls_logdir + + +pg_ls_logdir returns the last modified time (mtime), size +and names of all the file in the log directory. By default only superusers +can use this function, however additional roles may be granted execute +permission if required. + + + +pg_ls_waldir + + +pg_ls_waldir returns the last modified time (mtime), size +and names of all the file in the write ahead log (WAL) directory. By +default only superusers can use this function, however additional roles +may be granted execute permission if required. + + + pg_read_file diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 0bce20914e..b6552da4b0 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1102,3 +1102,6 @@ REVOKE EXECUTE ON FUNCTION pg_stat_reset() FROM public; REVOKE EXECUTE ON FUNCTION pg_stat_reset_shared(text) FROM public; REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_table_counters(oid) FROM public; REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_function_counters(oid) FROM public; + +REVOKE EXECUTE ON FUNCTION pg_ls_logdir() FROM public; +REVOKE EXECUTE ON FUNCTION pg_ls_waldir() FROM public; diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c index 1ec7f32470..ad75d18a2f 100644 --- a/src/backend/utils/adt/misc.c +++ b/src/backend/utils/adt/misc.c @@ -15,12 +15,14 @@ #include "postgres.h" #include +#include #include #include #include #include #include "access/sysattr.h" +#include "access/xlog_internal.h" #include "catalog/pg_authid.h" #include "catalog/catalog.h" #include "catalog/pg_tablespace.h" @@ -44,6 +46,8 @@ #include "utils/builtins.h" #include "utils/timestamp.h" +/* Generic function to return a directory listing of files */ +Datum pg_ls_dir_files(FunctionCallInfo fcinfo, char *dir); /* * Common subroutine for num_nulls() and num_nonnulls(). @@ -982,3 +986,111 @@ pg_current_logfile_1arg(PG_FUNCTION_ARGS) { return pg_current_logfile(fcinfo); } + + +typedef struct +{ + char*location; + DIR *dirdesc; +} directory_fctx; + +/* Generic function to return a directory listing of files */ +Datum +pg_ls_dir_files(FunctionCallInfo fcinfo, char *dir) +{ + FuncCallContext *funcctx; + struct dirent *de; + directory_fctx *fctx; + + if (SRF_IS_FIRSTCALL()) + { +
Re: [HACKERS] pg_ls_waldir() & pg_ls_logdir()
On Thu, Mar 16, 2017 at 10:40 PM, Dave Page wrote: >> +const int n = snprintf(NULL, 0, "%lld", attrib.st_size); I wonder what the portable printf directive is for off_t. Maybe better to use INT64_FORMAT and cast to int64? -- Thomas Munro http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_ls_waldir() & pg_ls_logdir()
Hi On Wed, Mar 15, 2017 at 5:27 PM, Robert Haas wrote: > On Mon, Feb 20, 2017 at 6:21 AM, Dave Page wrote: >> Patch includes the code and doc updates. > > Review: > > +strftime(mtime, 25, "%Y-%m-%d %H:%M:%S %Z", > localtime(&(attrib.st_ctime))); > +const int n = snprintf(NULL, 0, "%lld", attrib.st_size); > +char size[n+1]; > +snprintf(size, n+1, "%lld", attrib.st_size); > > We don't allow variable declarations in mid-block. You've been > programming in C++ for too long! Err, yeah. Ooops. Fixed. > The documentation should be updated to say that access to > pg_ls_logdir() and pg_ls_waldir() can be granted via the permissions > system (see the paragraph above the table you updated). > > The four existing functions in the documentation table each have a > corresponding paragraph below the table, but the two new functions you > added don't. Added. > +1 for the concept. Thanks, updated patch attached. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index a521912317..e15ad77dec 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -19646,7 +19646,8 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); database cluster directory and the log_directory can be accessed. Use a relative path for files in the cluster directory, and a path matching the log_directory configuration setting -for log files. Use of these functions is restricted to superusers. +for log files. Use of these functions is restricted to superusers +except where stated otherwise. @@ -19669,6 +19670,26 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); +pg_ls_logdir() + + setof record + +List the name, last modification time and size of files in the log directory. +Execute permission may be granted to non-superuser roles. + + + + +pg_ls_waldir() + + setof record + +List the name, last modification time and size of files in the WAL directory. +Execute permission may be granted to non-superuser roles. + + + + pg_read_file(filename text [, offset bigint, length bigint [, missing_ok boolean] ]) text @@ -19699,7 +19720,7 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); -All of these functions take an optional missing_ok parameter, +Some of these functions take an optional missing_ok parameter, which specifies the behavior when the file or directory does not exist. If true, the function returns NULL (except pg_ls_dir, which returns an empty result set). If @@ -19720,6 +19741,26 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); +pg_ls_logdir + + +pg_ls_logdir returns the last modified time (mtime), size +and names of all the file in the log directory. By default only superusers +can use this function, however additional roles may be granted execute +permission if required. + + + +pg_ls_waldir + + +pg_ls_waldir returns the last modified time (mtime), size +and names of all the file in the write ahead log (WAL) directory. By +default only superusers can use this function, however additional roles +may be granted execute permission if required. + + + pg_read_file diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 0bce20914e..b6552da4b0 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1102,3 +1102,6 @@ REVOKE EXECUTE ON FUNCTION pg_stat_reset() FROM public; REVOKE EXECUTE ON FUNCTION pg_stat_reset_shared(text) FROM public; REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_table_counters(oid) FROM public; REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_function_counters(oid) FROM public; + +REVOKE EXECUTE ON FUNCTION pg_ls_logdir() FROM public; +REVOKE EXECUTE ON FUNCTION pg_ls_waldir() FROM public; diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c index 1ec7f32470..2fadad860b 100644 --- a/src/backend/utils/adt/misc.c +++ b/src/backend/utils/adt/misc.c @@ -15,12 +15,14 @@ #include "postgres.h" #include +#include #include #include #include #include #include "access/sysattr.h" +#include "access/xlog_internal.h" #include "catalog/pg_authid.h" #include "catalog/catalog.h" #include "catalog/pg_tablespace.h" @@ -982,3 +984,111 @@ pg_current_logfile_1arg(PG_FUNCTION_ARGS) { return pg_current_logfile(fcinfo); } + + +typedef struct +{ + char*location; + DIR *dirdesc; +} directory_fctx; + +/* Generic function to return a directory listing of files */ +Datum +p
Re: [HACKERS] pg_ls_waldir() & pg_ls_logdir()
On Mon, Feb 20, 2017 at 6:21 AM, Dave Page wrote: > Patch includes the code and doc updates. Review: +strftime(mtime, 25, "%Y-%m-%d %H:%M:%S %Z", localtime(&(attrib.st_ctime))); +const int n = snprintf(NULL, 0, "%lld", attrib.st_size); +char size[n+1]; +snprintf(size, n+1, "%lld", attrib.st_size); We don't allow variable declarations in mid-block. You've been programming in C++ for too long! The documentation should be updated to say that access to pg_ls_logdir() and pg_ls_waldir() can be granted via the permissions system (see the paragraph above the table you updated). The four existing functions in the documentation table each have a corresponding paragraph below the table, but the two new functions you added don't. +1 for the concept. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_ls_waldir() & pg_ls_logdir()
Hi Following various conversations on list and in person, including the developer meeting in Brussels earlier this month, here is a patch that implements pg_ls_logdir() and pg_ls_waldir() functions. The ultimate aim of this (and followup work I'll be doing) is to provide functionality to enable monitoring of PostgreSQL without requiring a user with superuser permissions as many of us have users for whom security policies prevent this or make it very difficult. In order to achieve that, there are various pieces of functionality such as pg_ls_dir() that need to have superuser checks removed to allow permissions to be granted to a monitoring role. There were objections in previous discussions to doing this with such generic functions, hence this patch which adds two narrowly focussed functions to allow tools to monitor the contents of the log and WAL directories. Neither function has a hard-coded superuser check, but have ACLs that prevent public execution by default. Patch includes the code and doc updates. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index d7738b18b7..ecd17a3528 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -19360,6 +19360,24 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); +pg_ls_logdir() + + setof record + +List the name, last modification time and size of files in the log directory. + + + + +pg_ls_waldir() + + setof record + +List the name, last modification time and size of files in the WAL directory. + + + + pg_read_file(filename text [, offset bigint, length bigint [, missing_ok boolean] ]) text @@ -19390,7 +19408,7 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); -All of these functions take an optional missing_ok parameter, +Some of these functions take an optional missing_ok parameter, which specifies the behavior when the file or directory does not exist. If true, the function returns NULL (except pg_ls_dir, which returns an empty result set). If diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 38be9cf1a0..4b67102439 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1096,3 +1096,6 @@ REVOKE EXECUTE ON FUNCTION pg_stat_reset() FROM public; REVOKE EXECUTE ON FUNCTION pg_stat_reset_shared(text) FROM public; REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_table_counters(oid) FROM public; REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_function_counters(oid) FROM public; + +REVOKE EXECUTE ON FUNCTION pg_ls_logdir() FROM public; +REVOKE EXECUTE ON FUNCTION pg_ls_waldir() FROM public; diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c index 66d09bcb0c..a8cdf3bcbf 100644 --- a/src/backend/utils/adt/misc.c +++ b/src/backend/utils/adt/misc.c @@ -15,12 +15,14 @@ #include "postgres.h" #include +#include #include #include #include #include #include "access/sysattr.h" +#include "access/xlog_internal.h" #include "catalog/pg_authid.h" #include "catalog/catalog.h" #include "catalog/pg_tablespace.h" @@ -46,6 +48,8 @@ #define atooid(x) ((Oid) strtoul((x), NULL, 10)) +/* Generic function to return a directory listing of files */ +Datum pg_ls_dir_files(FunctionCallInfo fcinfo, char *dir); /* * Common subroutine for num_nulls() and num_nonnulls(). @@ -892,3 +896,109 @@ parse_ident(PG_FUNCTION_ARGS) PG_RETURN_DATUM(makeArrayResult(astate, CurrentMemoryContext)); } + + +typedef struct +{ + char*location; + DIR *dirdesc; +} directory_fctx; + +/* Generic function to return a directory listing of files */ +Datum +pg_ls_dir_files(FunctionCallInfo fcinfo, char *dir) +{ + FuncCallContext *funcctx; + struct dirent *de; + directory_fctx *fctx; + + if (SRF_IS_FIRSTCALL()) + { + MemoryContext oldcontext; + TupleDesc tupdesc; + + funcctx = SRF_FIRSTCALL_INIT(); + oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx); + + fctx = palloc(sizeof(directory_fctx)); + + tupdesc = CreateTemplateTupleDesc(3, false); + TupleDescInitEntry(tupdesc, (AttrNumber) 1, "mtime", + TIMESTAMPTZOID, -1, 0); + TupleDescInitEntry(tupdesc, (AttrNumber) 2, "size", + INT8OID, -1, 0); + TupleDescInitEntry(tupdesc, (AttrNumber) 3, "file", + TEXTOID, -1, 0); + + funcctx->attinmeta = TupleDescGe