Monitoring roles patch

Hi Dave,

The patch failed applied...

patch -p1 < /home/vagrant/pg_monitor.diff
patching file contrib/pg_buffercache/Makefile
patching file contrib/pg_buffercache/pg_buffercache--1.2--1.3.sql
patching file contrib/pg_buffercache/pg_buffercache.control
patching file contrib/pg_freespacemap/Makefile
patching file contrib/pg_freespacemap/pg_freespacemap--1.1--1.2.sql
patching file contrib/pg_freespacemap/pg_freespacemap.control
patching file contrib/pg_stat_statements/Makefile
patching file contrib/pg_stat_statements/pg_stat_statements--1.4--1.5.sql
patching file contrib/pg_stat_statements/pg_stat_statements.c
patching file contrib/pg_stat_statements/pg_stat_statements.control
patching file contrib/pg_visibility/Makefile
Hunk #1 succeeded at 4 with fuzz 1.
patching file contrib/pg_visibility/pg_visibility--1.1--1.2.sql
patching file contrib/pg_visibility/pg_visibility.control
patching file contrib/pgrowlocks/pgrowlocks.c
patching file contrib/pgstattuple/pgstattuple--1.4--1.5.sql
patching file doc/src/sgml/catalogs.sgml
Hunk #1 succeeded at 10027 (offset 11 lines).
patching file doc/src/sgml/func.sgml
Hunk #1 succeeded at 19364 (offset 311 lines).
Hunk #2 succeeded at 19648 (offset 311 lines).
patching file doc/src/sgml/pgbuffercache.sgml
patching file doc/src/sgml/pgfreespacemap.sgml
patching file doc/src/sgml/pgrowlocks.sgml
patching file doc/src/sgml/pgstatstatements.sgml
patching file doc/src/sgml/pgstattuple.sgml
patching file doc/src/sgml/pgvisibility.sgml
patching file doc/src/sgml/user-manag.sgml
patching file src/backend/catalog/system_views.sql
Hunk #1 FAILED at 1099.
1 out of 1 hunk FAILED -- saving rejects to file src/backend/catalog/system_views.sql.rej
patching file src/backend/replication/walreceiver.c
patching file src/backend/utils/adt/dbsize.c
Hunk #1 succeeded at 17 (offset -1 lines).
Hunk #2 succeeded at 89 (offset -1 lines).
Hunk #3 succeeded at 179 (offset -1 lines).
patching file src/backend/utils/adt/pgstatfuncs.c
patching file src/backend/utils/misc/guc.c
Hunk #2 succeeded at 6678 (offset 10 lines).
Hunk #3 succeeded at 6728 (offset 10 lines).
Hunk #4 succeeded at 8021 (offset 10 lines).
Hunk #5 succeeded at 8053 (offset 10 lines).
patching file src/include/catalog/pg_authid.h

Reject file contents...

cat src/backend/catalog/system_views.sql.rej
--- src/backend/catalog/system_views.sql
+++ src/backend/catalog/system_views.sql
@@ -1099,3 +1099,7 @@

 REVOKE EXECUTE ON FUNCTION pg_ls_logdir() FROM public;
 REVOKE EXECUTE ON FUNCTION pg_ls_waldir() FROM public;
+GRANT EXECUTE ON FUNCTION pg_ls_logdir() TO pg_monitor;
+GRANT EXECUTE ON FUNCTION pg_ls_waldir() TO pg_monitor;
+
+GRANT pg_read_all_gucs TO pg_monitor;

The new status of this patch is: Waiting on Author

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi

On Thu, Mar 16, 2017 at 7:04 PM, Denish Patel <denish.j.patel@gmail.com> wrote:

Hi Dave,

The patch failed applied...

patch -p1 < /home/vagrant/pg_monitor.diff
patching file contrib/pg_buffercache/Makefile
patching file contrib/pg_buffercache/pg_buffercache--1.2--1.3.sql
patching file contrib/pg_buffercache/pg_buffercache.control
patching file contrib/pg_freespacemap/Makefile
patching file contrib/pg_freespacemap/pg_freespacemap--1.1--1.2.sql
patching file contrib/pg_freespacemap/pg_freespacemap.control
patching file contrib/pg_stat_statements/Makefile
patching file contrib/pg_stat_statements/pg_stat_statements--1.4--1.5.sql
patching file contrib/pg_stat_statements/pg_stat_statements.c
patching file contrib/pg_stat_statements/pg_stat_statements.control
patching file contrib/pg_visibility/Makefile
Hunk #1 succeeded at 4 with fuzz 1.
patching file contrib/pg_visibility/pg_visibility--1.1--1.2.sql
patching file contrib/pg_visibility/pg_visibility.control
patching file contrib/pgrowlocks/pgrowlocks.c
patching file contrib/pgstattuple/pgstattuple--1.4--1.5.sql
patching file doc/src/sgml/catalogs.sgml
Hunk #1 succeeded at 10027 (offset 11 lines).
patching file doc/src/sgml/func.sgml
Hunk #1 succeeded at 19364 (offset 311 lines).
Hunk #2 succeeded at 19648 (offset 311 lines).
patching file doc/src/sgml/pgbuffercache.sgml
patching file doc/src/sgml/pgfreespacemap.sgml
patching file doc/src/sgml/pgrowlocks.sgml
patching file doc/src/sgml/pgstatstatements.sgml
patching file doc/src/sgml/pgstattuple.sgml
patching file doc/src/sgml/pgvisibility.sgml
patching file doc/src/sgml/user-manag.sgml
patching file src/backend/catalog/system_views.sql
Hunk #1 FAILED at 1099.
1 out of 1 hunk FAILED -- saving rejects to file src/backend/catalog/system_views.sql.rej
patching file src/backend/replication/walreceiver.c
patching file src/backend/utils/adt/dbsize.c
Hunk #1 succeeded at 17 (offset -1 lines).
Hunk #2 succeeded at 89 (offset -1 lines).
Hunk #3 succeeded at 179 (offset -1 lines).
patching file src/backend/utils/adt/pgstatfuncs.c
patching file src/backend/utils/misc/guc.c
Hunk #2 succeeded at 6678 (offset 10 lines).
Hunk #3 succeeded at 6728 (offset 10 lines).
Hunk #4 succeeded at 8021 (offset 10 lines).
Hunk #5 succeeded at 8053 (offset 10 lines).
patching file src/include/catalog/pg_authid.h

Reject file contents...

cat src/backend/catalog/system_views.sql.rej
--- src/backend/catalog/system_views.sql
+++ src/backend/catalog/system_views.sql
@@ -1099,3 +1099,7 @@

REVOKE EXECUTE ON FUNCTION pg_ls_logdir() FROM public;
REVOKE EXECUTE ON FUNCTION pg_ls_waldir() FROM public;
+GRANT EXECUTE ON FUNCTION pg_ls_logdir() TO pg_monitor;
+GRANT EXECUTE ON FUNCTION pg_ls_waldir() TO pg_monitor;
+
+GRANT pg_read_all_gucs TO pg_monitor;

The new status of this patch is: Waiting on Author

Odd - I get very different rejects than you. Perhaps you didn't apply
my pg_ls_logdir() patch first? In any case, that was committed last
night.

Here's a rebased patch.

Thanks!

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: tested, passed

Looks good.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2/24/17 05:14, Dave Page wrote:

- Adds a default role called pg_read_all_gucs
- Allows members of pg_read_all_gucs to, well, read all GUCs
- Grants pg_read_all_gucs to pg_monitor

Maybe pg_read_all_settings? Consistent with pg_settings.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Feb 24, 2017 at 5:14 AM, Dave Page <dpage@pgadmin.org> wrote:

- Adds a default role called pg_monitor
- Gives members of the pg_monitor role full access to:
pg_ls_logdir() and pg_ls_waldir()
pg_stat_* views and functions
pg_tablespace_size() and pg_database_size()
Contrib modules:
pg_buffercache,
pg_freespacemap,
pgrowlocks,
pg_stat_statements,
pgstattuple and
pg_visibility (but NOT pg_truncate_visibility_map() )
- Adds a default role called pg_read_all_gucs
- Allows members of pg_read_all_gucs to, well, read all GUCs
- Grants pg_read_all_gucs to pg_monitor

I like the pg_read_all_gucs role, which I agree with Peter should be
called pg_read_all_settings. I'd be inclined to skip the rest of
this. If an individual user wants to grant that bundle of privileges
to a role, they can do it with or without pg_monitor.

--
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

On Wed, Mar 22, 2017 at 11:32 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Feb 24, 2017 at 5:14 AM, Dave Page <dpage@pgadmin.org> wrote:

- Adds a default role called pg_monitor
- Gives members of the pg_monitor role full access to:
pg_ls_logdir() and pg_ls_waldir()
pg_stat_* views and functions
pg_tablespace_size() and pg_database_size()
Contrib modules:
pg_buffercache,
pg_freespacemap,
pgrowlocks,
pg_stat_statements,
pgstattuple and
pg_visibility (but NOT pg_truncate_visibility_map() )
- Adds a default role called pg_read_all_gucs
- Allows members of pg_read_all_gucs to, well, read all GUCs
- Grants pg_read_all_gucs to pg_monitor

I like the pg_read_all_gucs role, which I agree with Peter should be
called pg_read_all_settings.

No objection to that change.

I'd be inclined to skip the rest of
this. If an individual user wants to grant that bundle of privileges
to a role, they can do it with or without pg_monitor.

GRANT cannot be used in all cases, as some of the functions changed
have hard-coded superuser checks. In those cases, I've added
pg_monitor membership to the permission checks in the C code.

The reason for having the role is to minimise the amount of work
required by the user to setup a role for the purposes of monitoring
the server. This is a practice which is seen in other DBMSs for
usability.

With the patch, complex monitoring systems can easily be setup with
something like:

CREATE ROLE monitoring_user LOGIN;
GRANT pg_monitor TO monitoring_role;

Without, the users setting up their monitoring system will have to run
a much more complex set of GRANTs, almost certainly requiring
scripting.

--
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

On Wed, Mar 22, 2017 at 7:48 AM, Dave Page <dpage@pgadmin.org> wrote:

I'd be inclined to skip the rest of
this. If an individual user wants to grant that bundle of privileges
to a role, they can do it with or without pg_monitor.

GRANT cannot be used in all cases, as some of the functions changed
have hard-coded superuser checks. In those cases, I've added
pg_monitor membership to the permission checks in the C code.

Oh. Well, why not just control access using the permissions checking
mechanism entirely, without hardcoding any OID?

The reason for having the role is to minimise the amount of work
required by the user to setup a role for the purposes of monitoring
the server. This is a practice which is seen in other DBMSs for
usability.

With the patch, complex monitoring systems can easily be setup with
something like:

CREATE ROLE monitoring_user LOGIN;
GRANT pg_monitor TO monitoring_role;

Without, the users setting up their monitoring system will have to run
a much more complex set of GRANTs, almost certainly requiring
scripting.

But that script is easy to provide, probably not very long, and could
be bundled in an extension if it's helpful. I'm wary of committing
ourselves to a specific decision about what pg_monitor includes; that
seems like it could result in endless future litigation every time
somebody wants to make a change. In contrast, nobody's going to have
any question at all about the remit of pg_read_all_settings.

--
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

On 3/22/17 07:48, Dave Page wrote:

With the patch, complex monitoring systems can easily be setup with
something like:

CREATE ROLE monitoring_user LOGIN;
GRANT pg_monitor TO monitoring_role;

That assumes that we have thought of all the ways in which people might
want to monitor things.

If we do it via GRANTs instead, then users can easily extend it.

If we instead change the hardcoded superuser checks to hardcoded
some-other-role checks, then the whole system instantly becomes unusable
the moment someone wants to monitor something we haven't thought of.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

* Robert Haas (robertmhaas@gmail.com) wrote:

On Wed, Mar 22, 2017 at 7:48 AM, Dave Page <dpage@pgadmin.org> wrote:

I'd be inclined to skip the rest of
this. If an individual user wants to grant that bundle of privileges
to a role, they can do it with or without pg_monitor.

GRANT cannot be used in all cases, as some of the functions changed
have hard-coded superuser checks. In those cases, I've added
pg_monitor membership to the permission checks in the C code.

Oh. Well, why not just control access using the permissions checking
mechanism entirely, without hardcoding any OID?

I've not had the chance yet to review this (though I have been planning
to), but at least in most cases where we have hard-coded checks it's
because the GRANT system doesn't allow the kind of fine-grained access
we've implemented at the C level. pg_stat_activity is a great example
of this where, if you aren't a superuser or a member of the other role,
you can still query the table but you can't see the 'query' column for
those other connections.

I did specifically ask for explicit roles to be made to enable such
capability and that the pg_monitor role be GRANT'd those roles instead
of hacking the pg_monitor OID into those checks, but it seems like
that's not been done yet.

The reason for having the role is to minimise the amount of work
required by the user to setup a role for the purposes of monitoring
the server. This is a practice which is seen in other DBMSs for
usability.

With the patch, complex monitoring systems can easily be setup with
something like:

CREATE ROLE monitoring_user LOGIN;
GRANT pg_monitor TO monitoring_role;

Without, the users setting up their monitoring system will have to run
a much more complex set of GRANTs, almost certainly requiring
scripting.

But that script is easy to provide, probably not very long, and could
be bundled in an extension if it's helpful. I'm wary of committing
ourselves to a specific decision about what pg_monitor includes; that
seems like it could result in endless future litigation every time
somebody wants to make a change. In contrast, nobody's going to have
any question at all about the remit of pg_read_all_settings.

I tend to agree with Dave on this. While I understand this concern and
risk of litigation, I believe that comes down to, really, making sure
that we spell out exactly what pg_monitor is intended for and that the
privileges it has will change over time. If users are not comfortable
with that, then they can use provided script to create their own
'monitor' role- that is specifically why I want to have nothing
hard-coded to pg_monitor though, because otherwise you couldn't have
such a script able to GRANT out a subset of what pg_monitor allows.

Thanks!

Stephen

Peter,

* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:

On 3/22/17 07:48, Dave Page wrote:

With the patch, complex monitoring systems can easily be setup with
something like:

CREATE ROLE monitoring_user LOGIN;
GRANT pg_monitor TO monitoring_role;

That assumes that we have thought of all the ways in which people might
want to monitor things.

I disagree. The entire point of the pg_monitor role is to cover those
rights which we feel should be available to monitoring solutions, and
that *will* change over time.

If we do it via GRANTs instead, then users can easily extend it.

The intent here is that users will *also* be able to do it via GRANTs if
they wish to.

If we instead change the hardcoded superuser checks to hardcoded
some-other-role checks, then the whole system instantly becomes unusable
the moment someone wants to monitor something we haven't thought of.

Right, that's why we need specific roles for the cases where we have to
have a C-level check and the pg_monitor role should only be GRANT'd
those other roles or GRANTs on specific functions, all of which a
DBA/superuser could do themselves with their own role, if they wished to
do so, instead of using pg_monitor.

Thanks!

Stephen

On Wed, Mar 22, 2017 at 12:55 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 3/22/17 07:48, Dave Page wrote:

With the patch, complex monitoring systems can easily be setup with
something like:

CREATE ROLE monitoring_user LOGIN;
GRANT pg_monitor TO monitoring_role;

That assumes that we have thought of all the ways in which people might
want to monitor things.

Right - it was discussed here, and at other meetings. We may not have
everything but either users can GRANT anything we need that we missed
later, or we can add them in a future release.

If we do it via GRANTs instead, then users can easily extend it.

They can do that anyway.

If we instead change the hardcoded superuser checks to hardcoded
some-other-role checks, then the whole system instantly becomes unusable
the moment someone wants to monitor something we haven't thought of.

I haven't replaced the checks, I've made them superuser || pg_monitor.
Nothing is going to break if we haven't thought of something.

--
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

On Wed, Mar 22, 2017 at 1:15 PM, Stephen Frost <sfrost@snowman.net> wrote:

I did specifically ask for explicit roles to be made to enable such
capability and that the pg_monitor role be GRANT'd those roles instead
of hacking the pg_monitor OID into those checks, but it seems like
that's not been done yet.

Yeah, sorry - I missed that for pg_stat_activity. I'll update the patch.

--
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

On Wed, Mar 22, 2017 at 3:46 PM, Dave Page <dpage@pgadmin.org> wrote:

On Wed, Mar 22, 2017 at 1:15 PM, Stephen Frost <sfrost@snowman.net> wrote:

I did specifically ask for explicit roles to be made to enable such
capability and that the pg_monitor role be GRANT'd those roles instead
of hacking the pg_monitor OID into those checks, but it seems like
that's not been done yet.

Yeah, sorry - I missed that for pg_stat_activity. I'll update the patch.

Updated patch attached. This changes pg_read_all_gucs to
pg_read_all_settings, and adds pg_read_all_stats which it grants to
pg_monitor. pg_read_all_stats has full access to the pg_stat_ views
(as pg_monitor did previously), and is used in the various contrib
modules in place of pg_monitor.

Thanks.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Dave,

* Dave Page (dpage@pgadmin.org) wrote:

On Wed, Mar 22, 2017 at 3:46 PM, Dave Page <dpage@pgadmin.org> wrote:

On Wed, Mar 22, 2017 at 1:15 PM, Stephen Frost <sfrost@snowman.net> wrote:

I did specifically ask for explicit roles to be made to enable such
capability and that the pg_monitor role be GRANT'd those roles instead
of hacking the pg_monitor OID into those checks, but it seems like
that's not been done yet.

Yeah, sorry - I missed that for pg_stat_activity. I'll update the patch.

Updated patch attached. This changes pg_read_all_gucs to
pg_read_all_settings, and adds pg_read_all_stats which it grants to
pg_monitor. pg_read_all_stats has full access to the pg_stat_ views
(as pg_monitor did previously), and is used in the various contrib
modules in place of pg_monitor.

Thanks! I've started looking through it.

diff --git a/contrib/pg_buffercache/Makefile b/contrib/pg_buffercache/Makefile
index 497dbeb229..18f7a87452 100644
--- a/contrib/pg_buffercache/Makefile
+++ b/contrib/pg_buffercache/Makefile
@@ -4,8 +4,9 @@ MODULE_big = pg_buffercache
OBJS = pg_buffercache_pages.o $(WIN32RES)

EXTENSION = pg_buffercache
-DATA = pg_buffercache--1.2.sql pg_buffercache--1.1--1.2.sql \
-	pg_buffercache--1.0--1.1.sql pg_buffercache--unpackaged--1.0.sql
+DATA = pg_buffercache--1.2.sql pg_buffercache--1.2--1.3.sql \
+	pg_buffercache--1.1--1.2.sql pg_buffercache--1.0--1.1.sql \
+	pg_buffercache--unpackaged--1.0.sql
PGFILEDESC = "pg_buffercache - monitoring of shared buffer cache in real-time"

ifdef USE_PGXS

Did you forget to 'add' the new files..? I don't see the
pg_buffercache--1.2--1.3.sql file.

diff --git a/contrib/pg_freespacemap/Makefile b/contrib/pg_freespacemap/Makefile
index 7bc0e9555d..0a2f000ec6 100644
--- a/contrib/pg_freespacemap/Makefile
+++ b/contrib/pg_freespacemap/Makefile
@@ -4,8 +4,8 @@ MODULE_big = pg_freespacemap
OBJS = pg_freespacemap.o $(WIN32RES)

EXTENSION = pg_freespacemap
-DATA = pg_freespacemap--1.1.sql pg_freespacemap--1.0--1.1.sql \
-	pg_freespacemap--unpackaged--1.0.sql
+DATA = pg_freespacemap--1.1.sql pg_freespacemap--1.1--1.2.sql \
+	pg_freespacemap--1.0--1.1.sql pg_freespacemap--unpackaged--1.0.sql
PGFILEDESC = "pg_freespacemap - monitoring of free space map"

ifdef USE_PGXS

Same here.

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 298951a5f5..39b368b70e 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -4,9 +4,10 @@ MODULE_big = pg_stat_statements
OBJS = pg_stat_statements.o $(WIN32RES)

EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.3--1.4.sql \
-	pg_stat_statements--1.2--1.3.sql pg_stat_statements--1.1--1.2.sql \
-	pg_stat_statements--1.0--1.1.sql pg_stat_statements--unpackaged--1.0.sql
+DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.4--1.5.sql \
+	pg_stat_statements--1.3--1.4.sql pg_stat_statements--1.2--1.3.sql \
+	pg_stat_statements--1.1--1.2.sql pg_stat_statements--1.0--1.1.sql \
+	pg_stat_statements--unpackaged--1.0.sql
PGFILEDESC = "pg_stat_statements - execution statistics of SQL statements"

LDFLAGS_SL += $(filter -lm, $(LIBS))

And here.

diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 221ac98d4a..cec94d5896 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -1386,7 +1387,7 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
MemoryContext per_query_ctx;
MemoryContext oldcontext;
Oid			userid = GetUserId();
-	bool		is_superuser = superuser();
+	bool		is_superuser = false;
char	   *qbuffer = NULL;
Size		qbuffer_size = 0;
Size		extent = 0;
@@ -1394,6 +1395,9 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
HASH_SEQ_STATUS hash_seq;
pgssEntry  *entry;

+	/* For the purposes of this module, we consider pg_read_all_stats members to be superusers */
+	is_superuser = (superuser() || is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS));

Whoahhhh, let's *not* set an 'is_superuser' flag for a case where the
user is not, actually, a superuser. This should be 'allowed_role' or
something along those lines, I'm thinking.

diff --git a/contrib/pg_visibility/Makefile b/contrib/pg_visibility/Makefile
index bc42944426..21d787ddf7 100644
--- a/contrib/pg_visibility/Makefile
+++ b/contrib/pg_visibility/Makefile
@@ -4,7 +4,8 @@ MODULE_big = pg_visibility
OBJS = pg_visibility.o $(WIN32RES)

EXTENSION = pg_visibility
-DATA = pg_visibility--1.1.sql pg_visibility--1.0--1.1.sql
+DATA = pg_visibility--1.1.sql pg_visibility--1.1--1.2.sql \
+	pg_visibility--1.0--1.1.sql
PGFILEDESC = "pg_visibility - page visibility information"

REGRESS = pg_visibility

Missing file here also.

diff --git a/contrib/pgrowlocks/pgrowlocks.c b/contrib/pgrowlocks/pgrowlocks.c
index db9e0349a0..c9194d8c0d 100644
--- a/contrib/pgrowlocks/pgrowlocks.c
+++ b/contrib/pgrowlocks/pgrowlocks.c
@@ -28,6 +28,7 @@
#include "access/relscan.h"
#include "access/xact.h"
#include "catalog/namespace.h"
+#include "catalog/pg_authid.h"
#include "funcapi.h"
#include "miscadmin.h"
#include "storage/bufmgr.h"
@@ -98,9 +99,11 @@ pgrowlocks(PG_FUNCTION_ARGS)
relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
rel = heap_openrv(relrv, AccessShareLock);

-		/* check permissions: must have SELECT on table */
-		aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
-									  ACL_SELECT);
+		/* check permissions: must have SELECT on table or be in pg_read_all_stats */
+		aclresult = (pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
+									  ACL_SELECT) ||
+			is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS));
+
if (aclresult != ACLCHECK_OK)
aclcheck_error(aclresult, ACL_KIND_CLASS,
RelationGetRelationName(rel));

Doesn't this go beyond just pg_stat_X, but actually allows running
pgrowlocks against any relation? That seems like it should be
independent, though it actually fits the "global-read-metadata" role
case that has been discussed previously as being what pg_visibility,
pgstattuple, and other similar contrib modules need. I don't have a
great name for that role, but it seems different to me from
'pg_read_all_stats', though I don't hold that position very strongly as
I can see an argument that these kind of are stats.

I see you did change pgstattuple too, and perhaps these are the changes
you intended for pg_visibility too? In any case, if so, we need to make
it clear in the docs that pg_read_all_stats grants access to more than
just pg_stat_X.

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index df0435c3f0..5472ff76a7 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -10027,15 +10027,17 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
<entry><type>text</type></entry>
<entry>Configuration file the current value was set in (null for
values set from sources other than configuration files, or when
-      examined by a non-superuser);
-      helpful when using <literal>include</> directives in configuration files</entry>
+      examined by a user who is neither a superuser or a member of
+      <literal>pg_read_all_settings</literal>); helpful when using
+      <literal>include</> directives in configuration files</entry>
</row>
<row>
<entry><structfield>sourceline</structfield></entry>
<entry><type>integer</type></entry>
<entry>Line number within the configuration file the current value was
set at (null for values set from sources other than configuration files,
-      or when examined by a non-superuser)
+      or when examined by a user who is neither a superuser or a member of
+      <literal>pg_read_all_settings</literal>).
</entry>
</row>
<row>

We should really figure out a way to link back to the default roles
section when we refer to roles from other places. Not sure what the
best way to do that off-hand is though.

diff --git a/doc/src/sgml/pgbuffercache.sgml b/doc/src/sgml/pgbuffercache.sgml
index b261a4dbe0..9d1faefdd6 100644
--- a/doc/src/sgml/pgbuffercache.sgml
+++ b/doc/src/sgml/pgbuffercache.sgml
@@ -24,8 +24,9 @@
</para>

<para>
-  By default public access is revoked from both of these, just in case there
-  are security issues lurking.
+  By default use is restricted to superusers and members of the
+  <literal>pg_read_all_stats</literal> role, just in case there are security
+  issues lurking.
</para>

<sect2>
diff --git a/doc/src/sgml/pgfreespacemap.sgml b/doc/src/sgml/pgfreespacemap.sgml
index f2f99d571e..f4f0e08473 100644
--- a/doc/src/sgml/pgfreespacemap.sgml
+++ b/doc/src/sgml/pgfreespacemap.sgml
@@ -16,8 +16,9 @@
</para>

<para>
-  By default public access is revoked from the functions, just in case
-  there are security issues lurking.
+  By default use is restricted to superusers and members of the
+  <literal>pg_read_all_stats</literal> role, just in case there are security
+  issues lurking.
</para>

<sect2>

These also look like cases where we might want to think about if we
should have a dedicated role which, essentially, allows locking and
scanning of all relations.

In particular, I can certainly imagine admins who wish to GRANT out the
rights to view other queries in pg_stat_activity to another admin and
maybe even allow them to run pg_terminate_backend, or pg_cancel_query,
but not allow them to scan every relation in the database.

diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index 31c567b37e..8e5c8c507c 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -50,6 +50,7 @@
#include "access/timeline.h"
#include "access/transam.h"
#include "access/xlog_internal.h"
+#include "catalog/pg_authid.h"
#include "catalog/pg_type.h"
#include "funcapi.h"
#include "libpq/pqformat.h"
@@ -1396,7 +1397,7 @@ pg_stat_get_wal_receiver(PG_FUNCTION_ARGS)
/* Fetch values */
values[0] = Int32GetDatum(walrcv->pid);

-	if (!superuser())
+	if (!superuser() && !is_member_of_role(GetUserId(), DEFAULT_ROLE_MONITOR))
{
/*
* Only superusers can see details. Other users only get the pid value

Shouldn't that be DEFAULT_ROLE_READ_ALL_STATS, not MONITOR?

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 4feb26aa7a..d1da580c9c 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -34,6 +34,7 @@
#include "access/xact.h"
#include "access/xlog_internal.h"
#include "catalog/namespace.h"
+#include "catalog/pg_authid.h"
#include "commands/async.h"
#include "commands/prepare.h"
#include "commands/user.h"
@@ -6677,10 +6678,12 @@ GetConfigOption(const char *name, bool missing_ok, bool restrict_superuser)
}
if (restrict_superuser &&
(record->flags & GUC_SUPERUSER_ONLY) &&
-		!superuser())
+		!superuser() &&
+		!is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_SETTINGS))

Seems like having a variable to indicate if the caller should be able to
read all these settings or not might be nice to have, so we have one
place that figures out the privileges question.

A few comments where that variable is set wouldn't be bad either.

Also, I'm thinking the pg_monitor documentation really needs to be
expanded and made very clear. I'll think a bit more on just how to try
and phrase that, but the description included certainly seemed
inadequate.

That's it for a quick review. If we can get these changes done and
there aren't any more questions or concerns, I'm happy to continue
working to move this forward. I definitely see the usefulness of these
changes and it'd be great to have an answer to the oft-asked question of
how to do proper monitoring with a non-superuser role.

Thanks!

Stephen

Hi

On Thu, Mar 23, 2017 at 12:23 PM, Stephen Frost <sfrost@snowman.net> wrote:

diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 221ac98d4a..cec94d5896 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -1386,7 +1387,7 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
MemoryContext per_query_ctx;
MemoryContext oldcontext;
Oid                     userid = GetUserId();
-     bool            is_superuser = superuser();
+     bool            is_superuser = false;
char       *qbuffer = NULL;
Size            qbuffer_size = 0;
Size            extent = 0;
@@ -1394,6 +1395,9 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
HASH_SEQ_STATUS hash_seq;
pgssEntry  *entry;

+     /* For the purposes of this module, we consider pg_read_all_stats members to be superusers */
+     is_superuser = (superuser() || is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS));

Whoahhhh, let's *not* set an 'is_superuser' flag for a case where the
user is not, actually, a superuser. This should be 'allowed_role' or
something along those lines, I'm thinking.

Fixed.

diff --git a/contrib/pg_visibility/Makefile b/contrib/pg_visibility/Makefile
index bc42944426..21d787ddf7 100644
--- a/contrib/pg_visibility/Makefile
+++ b/contrib/pg_visibility/Makefile
@@ -4,7 +4,8 @@ MODULE_big = pg_visibility
OBJS = pg_visibility.o $(WIN32RES)

EXTENSION = pg_visibility
-DATA = pg_visibility--1.1.sql pg_visibility--1.0--1.1.sql
+DATA = pg_visibility--1.1.sql pg_visibility--1.1--1.2.sql \
+     pg_visibility--1.0--1.1.sql
PGFILEDESC = "pg_visibility - page visibility information"

REGRESS = pg_visibility

Missing file here also.

Yeah, I forgot to add this and the others. Sorry about - fixed now.

diff --git a/contrib/pgrowlocks/pgrowlocks.c b/contrib/pgrowlocks/pgrowlocks.c
index db9e0349a0..c9194d8c0d 100644
--- a/contrib/pgrowlocks/pgrowlocks.c
+++ b/contrib/pgrowlocks/pgrowlocks.c
@@ -28,6 +28,7 @@
#include "access/relscan.h"
#include "access/xact.h"
#include "catalog/namespace.h"
+#include "catalog/pg_authid.h"
#include "funcapi.h"
#include "miscadmin.h"
#include "storage/bufmgr.h"
@@ -98,9 +99,11 @@ pgrowlocks(PG_FUNCTION_ARGS)
relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
rel = heap_openrv(relrv, AccessShareLock);

-             /* check permissions: must have SELECT on table */
-             aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
-                                                                       ACL_SELECT);
+             /* check permissions: must have SELECT on table or be in pg_read_all_stats */
+             aclresult = (pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
+                                                                       ACL_SELECT) ||
+                     is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS));
+
if (aclresult != ACLCHECK_OK)
aclcheck_error(aclresult, ACL_KIND_CLASS,
RelationGetRelationName(rel));

Doesn't this go beyond just pg_stat_X, but actually allows running
pgrowlocks against any relation? That seems like it should be
independent, though it actually fits the "global-read-metadata" role
case that has been discussed previously as being what pg_visibility,
pgstattuple, and other similar contrib modules need. I don't have a
great name for that role, but it seems different to me from
'pg_read_all_stats', though I don't hold that position very strongly as
I can see an argument that these kind of are stats.

That was the way I was leaning, on the grounds that the distinction
between two separate roles might end up being confusing for users.
It's simple enough to change if that's the consencus, and we can come
up with a suitable name.

I see you did change pgstattuple too, and perhaps these are the changes
you intended for pg_visibility too? In any case, if so, we need to make
it clear in the docs that pg_read_all_stats grants access to more than
just pg_stat_X.

The pg_read_all_stats role docs have:

Read all pg_stat_* views and use various statistics related
extensions, even those normally visible only to superusers.

And then I updated the docs for each contrib module to note where
pg_read_all_stats can use functionality.

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index df0435c3f0..5472ff76a7 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -10027,15 +10027,17 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
<entry><type>text</type></entry>
<entry>Configuration file the current value was set in (null for
values set from sources other than configuration files, or when
-      examined by a non-superuser);
-      helpful when using <literal>include</> directives in configuration files</entry>
+      examined by a user who is neither a superuser or a member of
+      <literal>pg_read_all_settings</literal>); helpful when using
+      <literal>include</> directives in configuration files</entry>
</row>
<row>
<entry><structfield>sourceline</structfield></entry>
<entry><type>integer</type></entry>
<entry>Line number within the configuration file the current value was
set at (null for values set from sources other than configuration files,
-      or when examined by a non-superuser)
+      or when examined by a user who is neither a superuser or a member of
+      <literal>pg_read_all_settings</literal>).
</entry>
</row>
<row>

We should really figure out a way to link back to the default roles
section when we refer to roles from other places. Not sure what the
best way to do that off-hand is though.

Yeah, I have no idea about that.

diff --git a/doc/src/sgml/pgbuffercache.sgml b/doc/src/sgml/pgbuffercache.sgml
index b261a4dbe0..9d1faefdd6 100644
--- a/doc/src/sgml/pgbuffercache.sgml
+++ b/doc/src/sgml/pgbuffercache.sgml
@@ -24,8 +24,9 @@
</para>

<para>
-  By default public access is revoked from both of these, just in case there
-  are security issues lurking.
+  By default use is restricted to superusers and members of the
+  <literal>pg_read_all_stats</literal> role, just in case there are security
+  issues lurking.
</para>

<sect2>
diff --git a/doc/src/sgml/pgfreespacemap.sgml b/doc/src/sgml/pgfreespacemap.sgml
index f2f99d571e..f4f0e08473 100644
--- a/doc/src/sgml/pgfreespacemap.sgml
+++ b/doc/src/sgml/pgfreespacemap.sgml
@@ -16,8 +16,9 @@
</para>

<para>
-  By default public access is revoked from the functions, just in case
-  there are security issues lurking.
+  By default use is restricted to superusers and members of the
+  <literal>pg_read_all_stats</literal> role, just in case there are security
+  issues lurking.
</para>

<sect2>

These also look like cases where we might want to think about if we
should have a dedicated role which, essentially, allows locking and
scanning of all relations.

In particular, I can certainly imagine admins who wish to GRANT out the
rights to view other queries in pg_stat_activity to another admin and
maybe even allow them to run pg_terminate_backend, or pg_cancel_query,
but not allow them to scan every relation in the database.

Easy enough to add - thoughts on the name though? I'm struggling to
come up with anything sane.

diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index 31c567b37e..8e5c8c507c 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -50,6 +50,7 @@
#include "access/timeline.h"
#include "access/transam.h"
#include "access/xlog_internal.h"
+#include "catalog/pg_authid.h"
#include "catalog/pg_type.h"
#include "funcapi.h"
#include "libpq/pqformat.h"
@@ -1396,7 +1397,7 @@ pg_stat_get_wal_receiver(PG_FUNCTION_ARGS)
/* Fetch values */
values[0] = Int32GetDatum(walrcv->pid);

-     if (!superuser())
+     if (!superuser() && !is_member_of_role(GetUserId(), DEFAULT_ROLE_MONITOR))
{
/*
* Only superusers can see details. Other users only get the pid value

Shouldn't that be DEFAULT_ROLE_READ_ALL_STATS, not MONITOR?

Ooops, fixed.

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 4feb26aa7a..d1da580c9c 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -34,6 +34,7 @@
#include "access/xact.h"
#include "access/xlog_internal.h"
#include "catalog/namespace.h"
+#include "catalog/pg_authid.h"
#include "commands/async.h"
#include "commands/prepare.h"
#include "commands/user.h"
@@ -6677,10 +6678,12 @@ GetConfigOption(const char *name, bool missing_ok, bool restrict_superuser)
}
if (restrict_superuser &&
(record->flags & GUC_SUPERUSER_ONLY) &&
-             !superuser())
+             !superuser() &&
+             !is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_SETTINGS))

Seems like having a variable to indicate if the caller should be able to
read all these settings or not might be nice to have, so we have one
place that figures out the privileges question.

Those checks are not all identical, and they're in different functions
so a variable wouldn't work (I assume you're not suggesting I add a
global :-p ). I could push part of each check out into a separate
function, but it doesn't seem like it would really add to the
readability to me.

A few comments where that variable is set wouldn't be bad either.

Also, I'm thinking the pg_monitor documentation really needs to be
expanded and made very clear. I'll think a bit more on just how to try
and phrase that, but the description included certainly seemed
inadequate.

I've added some additional text below the table.

That's it for a quick review. If we can get these changes done and
there aren't any more questions or concerns, I'm happy to continue
working to move this forward. I definitely see the usefulness of these
changes and it'd be great to have an answer to the oft-asked question of
how to do proper monitoring with a non-superuser role.

Thanks - updated patch attached.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 3/22/17 09:17, Stephen Frost wrote:

If we do it via GRANTs instead, then users can easily extend it.

The intent here is that users will *also* be able to do it via GRANTs if
they wish to.

But why not do it with GRANTs in the first place then?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Peter,

* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:

On 3/22/17 09:17, Stephen Frost wrote:

If we do it via GRANTs instead, then users can easily extend it.

The intent here is that users will *also* be able to do it via GRANTs if
they wish to.

But why not do it with GRANTs in the first place then?

This is akin to asking why do we need GRANT ALL and ALTER DEFAULT PRIVs.

Would it be technically possible to make users jump through hoops every
time they set up a new system to create their own monitor role that
would then have the right set of privileges for *that* version of PG?
Yes, but it's not exactly friendly. The whole idea here is that the
tool authors will be able to tell the users that they just need to GRANT
this one role, not a script of 20 GRANT statements, oh, and that it
won't break when doing upgrades.

If we make the users run all the statements individually then they'll
also have to get an updated script for the next version of PG too
because we will have added things that the tools will want access to.

Further, they'll have to make sure to install all the contrib modules
they want to use before running the GRANT script which is provided, or
deal with GRANT'ing the rights out after installing some extension.

With the approach that Dave and I are advocating, we can avoid all of
that. Contrib modules can bake-in GRANTs to the appropriate roles,
upgrades can be handled smoothly even when we add new capabilities which
are appropriate, users have a simple and straight-forward way to set up
good monitoring, and tool authors will know what permissions are
available and can finally have a better answer than "well, just make the
monior user superuser if you want to avoid all these complexities."

Thanks!

Stephen

On Fri, Mar 24, 2017 at 8:30 AM, Stephen Frost <sfrost@snowman.net> wrote:

But why not do it with GRANTs in the first place then?

This is akin to asking why do we need GRANT ALL and ALTER DEFAULT PRIVs.

Not really. ALTER DEFAULT PRIVILEGES affects what happens for future
objects, which isn't necessary here at all. The monitoring tool only
needs to be granted the minimum set of privileges that it currently
needs, not future privileges which it or other monitoring tools may
need in future versions. GRANT ALL is closer, but GRANT .. ON ALL
TABLES IN SCHEMA really is just a convenience function. You would
still have the capability to do the exact same thing without that
function; it would just be more work. And the same is true here. I
understand why you and Dave want to make this a single command: that's
easier. But, like GRANT ON ALL TABLES, it's also less flexible. If
you want to grant on all tables except one, you can't use that magic
syntax any more, and so here. pg_monitor will either target one
particular monitoring solution (hey, maybe it'll be the one my
employer produces!) or the judgement of one particular person
(whatever Stephen Frost thinks it SHOULD do in a given release,
independent of what tools on the ground do) and those aren't good
definitions.

If we make the users run all the statements individually then they'll
also have to get an updated script for the next version of PG too
because we will have added things that the tools will want access to.

That's possible, but it really depends on the tool, not on core
PostgreSQL. The tool should be the one providing the script, because
the tool is what knows its own permissions requirements. Users who
care about security won't want to grant every privilege given by a
pg_monitor role to a tool that only needs a subset of those
privileges.

Further, they'll have to make sure to install all the contrib modules
they want to use before running the GRANT script which is provided, or
deal with GRANT'ing the rights out after installing some extension.

That doesn't sound very hard.

With the approach that Dave and I are advocating, we can avoid all of
that. Contrib modules can bake-in GRANTs to the appropriate roles,
upgrades can be handled smoothly even when we add new capabilities which
are appropriate, users have a simple and straight-forward way to set up
good monitoring, and tool authors will know what permissions are
available and can finally have a better answer than "well, just make the
monior user superuser if you want to avoid all these complexities."

They can have that anyway. They just have to run a script provided by
the tool rather than one provided by the core project as a
one-size-fits-all solution for every tool.

--
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

On Fri, Mar 24, 2017 at 4:24 PM, Robert Haas <robertmhaas@gmail.com> wrote:

If we make the users run all the statements individually then they'll
also have to get an updated script for the next version of PG too
because we will have added things that the tools will want access to.

That's possible, but it really depends on the tool, not on core
PostgreSQL. The tool should be the one providing the script, because
the tool is what knows its own permissions requirements. Users who
care about security won't want to grant every privilege given by a
pg_monitor role to a tool that only needs a subset of those
privileges.

The upshot of this would be that every time there's a database server
upgrade that changes the permissions required somehow, the user has to
login to every server they have and run a script. It is no longer a
one-time thing, which makes it vastly more painful to deal with
upgrades.

With the approach that Dave and I are advocating, we can avoid all of
that. Contrib modules can bake-in GRANTs to the appropriate roles,
upgrades can be handled smoothly even when we add new capabilities which
are appropriate, users have a simple and straight-forward way to set up
good monitoring, and tool authors will know what permissions are
available and can finally have a better answer than "well, just make the
monior user superuser if you want to avoid all these complexities."

They can have that anyway. They just have to run a script provided by
the tool rather than one provided by the core project as a
one-size-fits-all solution for every tool.

Do you object to having individual default roles specifically for
cases where there are superuser-only checks at the moment that prevent
GRANT? e.g. pg_show_all_settings for SHOW, pg_show_all_stats for
pg_tablespace_size and friends, pg_stat_statements for, well,
pg_stat_statements and so on? It would be an inferior solution in my
opinion, given that it would demonstrably cause users more work, but
at least we could do something.

--
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