Re: [HACKERS] Monitoring roles patch

2017-03-30 Thread Dave Page
On Thu, Mar 30, 2017 at 2:24 PM, Simon Riggs wrote: > On 30 March 2017 at 18:29, Simon Riggs wrote: > >> Moving to commit this over the next hour. Last chance... > > Done. Great work Dave, thanks everybody. Thanks Simon. -- Dave Page Blog:

Re: [HACKERS] Monitoring roles patch

2017-03-30 Thread Simon Riggs
On 30 March 2017 at 18:29, Simon Riggs wrote: > Moving to commit this over the next hour. Last chance... Done. Great work Dave, thanks everybody. -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training &

Re: [HACKERS] Monitoring roles patch

2017-03-30 Thread Simon Riggs
On 29 March 2017 at 21:42, Dave Page wrote: > On Wed, Mar 29, 2017 at 2:51 PM, Stephen Frost wrote: >> >> Dave's currently hacking on a new patch based on our discussion, so I'd >> suggest waiting another hour or so anyway until he's done. >> >> Might be a

Re: [HACKERS] Monitoring roles patch

2017-03-29 Thread Dave Page
On Wed, Mar 29, 2017 at 2:51 PM, Stephen Frost wrote: > > Dave's currently hacking on a new patch based on our discussion, so I'd > suggest waiting another hour or so anyway until he's done. > > Might be a bit longer as he's trying to do it in a hallway at > PGConf.US...

Re: [HACKERS] Monitoring roles patch

2017-03-29 Thread Stephen Frost
* Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote: > On 3/28/17 12:19, Dave Page wrote: > > On Tue, Mar 28, 2017 at 11:39 AM, Peter Eisentraut > > wrote: > >> On 3/28/17 11:34, Dave Page wrote: > >>> On Tue, Mar 28, 2017 at 11:31 AM, Peter Eisentraut >

Re: [HACKERS] Monitoring roles patch

2017-03-29 Thread Peter Eisentraut
On 3/28/17 12:19, Dave Page wrote: > On Tue, Mar 28, 2017 at 11:39 AM, Peter Eisentraut > wrote: >> On 3/28/17 11:34, Dave Page wrote: >>> On Tue, Mar 28, 2017 at 11:31 AM, Peter Eisentraut >>> wrote: This patch touches the

Re: [HACKERS] Monitoring roles patch

2017-03-29 Thread Stephen Frost
Dave, * Dave Page (dp...@pgadmin.org) wrote: > OK, so essentially what I did, except s/pg_read_all_stats/pg_read_all_queries > ? Yup. > So pgstattuple, pg_sfreespacemap, pg_visibility and pgrowlocks to be > allowed access from members of pg_stat_scan_tables, which in turn is > granted to

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Mark Dilger
> On Mar 28, 2017, at 1:17 PM, Tom Lane wrote: > > Mark Dilger writes: >> I don't see anything wrong with adding roles in pg_authid.h with a #define'd >> Oid. That's actually pretty helpful for anyone writing code against the >> database, >> as

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Tom Lane
Mark Dilger writes: > I don't see anything wrong with adding roles in pg_authid.h with a #define'd > Oid. That's actually pretty helpful for anyone writing code against the > database, > as they don't have to look up the Oid of the role. > But why not then grant

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Dave Page
Hi On Tue, Mar 28, 2017 at 2:22 PM, Stephen Frost wrote: > Dave, > > * Dave Page (dp...@pgadmin.org) wrote: >> OK, so before I start hacking again, here's a proposal based on my >> understanding of folks comments, and so open questions. If I can get >> agreement and answers,

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Mark Dilger
> On Mar 28, 2017, at 11:52 AM, Tom Lane wrote: > > Mark Dilger writes: >> After a bit of introspection, I think what is really bothering me is not the >> inability to revoke permissions, since as you say I can choose to not assign >> the role to

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Tom Lane
Mark Dilger writes: > After a bit of introspection, I think what is really bothering me is not the > inability to revoke permissions, since as you say I can choose to not assign > the role to anybody. What bothers me is that this feature implicitly > redefines > what is

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Stephen Frost
Greetings, * Mark Dilger (hornschnor...@gmail.com) wrote: > After a bit of introspection, I think what is really bothering me is not the > inability to revoke permissions, since as you say I can choose to not assign > the role to anybody. What bothers me is that this feature implicitly >

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Mark Dilger
> On Mar 28, 2017, at 11:06 AM, Mark Dilger wrote: > > >> On Mar 28, 2017, at 9:47 AM, Dave Page wrote: >> >>> Does >>> pg_read_all_stats still have access to stats for mysecuretable? >> >> Yes, because the ACL on the table controls

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Stephen Frost
Greetings, * Mark Dilger (hornschnor...@gmail.com) wrote: > The inability to revoke access to this sort of information being proposed > makes me a bit uneasy. What data are you concerned about, specifically? > Mostly, I think, I'm bothered because there may > be people who have revoked

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Stephen Frost
Dave, * Dave Page (dp...@pgadmin.org) wrote: > OK, so before I start hacking again, here's a proposal based on my > understanding of folks comments, and so open questions. If I can get > agreement and answers, I'll be able to break out vi again without > (hopefully) too many more revisions: > >

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Mark Dilger
> On Mar 28, 2017, at 9:47 AM, Dave Page wrote: > >> Does >> pg_read_all_stats still have access to stats for mysecuretable? > > Yes, because the ACL on the table controls reading/writing the data in > the table. It doesn't have any bearing on any kind of table metadata. > A

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Dave Page
On Tue, Mar 28, 2017 at 1:52 PM, Mark Dilger wrote: > >> On Mar 28, 2017, at 9:55 AM, Robert Haas wrote: >> >> On Tue, Mar 28, 2017 at 12:47 PM, Dave Page wrote: I don't see any precedent in the code for having a hardcoded

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Mark Dilger
> On Mar 28, 2017, at 9:55 AM, Robert Haas wrote: > > On Tue, Mar 28, 2017 at 12:47 PM, Dave Page wrote: >>> I don't see any precedent in the code for having a hardcoded role, other >>> than >>> superuser, and allowing privileges based on a hardcoded

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Dave Page
OK, so before I start hacking again, here's a proposal based on my understanding of folks comments, and so open questions. If I can get agreement and answers, I'll be able to break out vi again without (hopefully) too many more revisions: pg_read_all_stats: Will have C-coded access to pg_stats

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Stephen Frost
Greetings, * Robert Haas (robertmh...@gmail.com) wrote: > On Tue, Mar 28, 2017 at 12:47 PM, Dave Page wrote: > >> I don't see any precedent in the code for having a hardcoded role, other > >> than > >> superuser, and allowing privileges based on a hardcoded test for

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Dave Page
On Tue, Mar 28, 2017 at 12:55 PM, Robert Haas wrote: > On Tue, Mar 28, 2017 at 12:47 PM, Dave Page wrote: >>> I don't see any precedent in the code for having a hardcoded role, other >>> than >>> superuser, and allowing privileges based on a hardcoded

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Robert Haas
On Tue, Mar 28, 2017 at 12:47 PM, Dave Page wrote: >> I don't see any precedent in the code for having a hardcoded role, other than >> superuser, and allowing privileges based on a hardcoded test for membership >> in that role. I'm struggling to think of all the security

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Dave Page
On Tue, Mar 28, 2017 at 12:04 PM, Mark Dilger wrote: > >> On Mar 28, 2017, at 8:34 AM, Dave Page wrote: >> >> On Tue, Mar 28, 2017 at 11:31 AM, Peter Eisentraut >> wrote: >>> This patch touches the pg_buffercache and

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Dave Page
On Tue, Mar 28, 2017 at 11:39 AM, Peter Eisentraut wrote: > On 3/28/17 11:34, Dave Page wrote: >> On Tue, Mar 28, 2017 at 11:31 AM, Peter Eisentraut >> wrote: >>> This patch touches the pg_buffercache and pg_freespacemap

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Mark Dilger
> On Mar 28, 2017, at 8:34 AM, Dave Page wrote: > > On Tue, Mar 28, 2017 at 11:31 AM, Peter Eisentraut > wrote: >> This patch touches the pg_buffercache and pg_freespacemap extensions, >> but there appear to be some files missing. > > Are

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Peter Eisentraut
On 3/28/17 11:34, Dave Page wrote: > On Tue, Mar 28, 2017 at 11:31 AM, Peter Eisentraut > wrote: >> This patch touches the pg_buffercache and pg_freespacemap extensions, >> but there appear to be some files missing. > > Are you looking at an old version? There

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Peter Eisentraut
On 3/27/17 08:54, Robert Haas wrote: > OK, I see the points that both of you are making and I admit that > they've got some validity to them. Let me make a modest proposal. > Suppose we define the pg_monitor role as strictly a bundle of > privileges that could be granted individually if desired,

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Dave Page
On Tue, Mar 28, 2017 at 11:31 AM, Peter Eisentraut wrote: > This patch touches the pg_buffercache and pg_freespacemap extensions, > but there appear to be some files missing. Are you looking at an old version? There was one where I forgot to add some files, but

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Peter Eisentraut
This patch touches the pg_buffercache and pg_freespacemap extensions, but there appear to be some files missing. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Peter Eisentraut
On 3/28/17 10:24, Simon Riggs wrote: >> Let me make a modest proposal. > > +1 > > I aim to commit something today. If you have other viewpoints, so say > now. There aren't many days left to express views and do anything > about them, so lets do it. I don't even see an actual proposed patch

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Simon Riggs
On 27 March 2017 at 13:54, Robert Haas wrote: > On Sat, Mar 25, 2017 at 12:30 PM, Dave Page wrote: >> Right - and if a user doesn't like it, they can easily just not use the >> default role and create their own alternative instead. > > OK, I see the

Re: [HACKERS] Monitoring roles patch

2017-03-28 Thread Simon Riggs
On 23 March 2017 at 13:16, Dave Page wrote: > Thanks - updated patch attached. I've edited the comments and docs on this patch, so attach a new version with similar name. -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote

Re: [HACKERS] Monitoring roles patch

2017-03-27 Thread Robert Haas
On Sat, Mar 25, 2017 at 12:30 PM, Dave Page wrote: > Right - and if a user doesn't like it, they can easily just not use the > default role and create their own alternative instead. OK, I see the points that both of you are making and I admit that they've got some validity to

Re: [HACKERS] Monitoring roles patch

2017-03-27 Thread Dave Page
On Mon, Mar 27, 2017 at 3:51 AM, Simon Riggs wrote: > On 25 March 2017 at 16:30, Dave Page wrote: > >> I believe this and other reasons we've described are exactly why other DBMS' >> do what we're proposing. > > It would help review if you could show

Re: [HACKERS] Monitoring roles patch

2017-03-27 Thread Simon Riggs
On 23 March 2017 at 13:16, Dave Page wrote: > Thanks - updated patch attached. No problems with the patch so far. I'd like some tests though... -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] Monitoring roles patch

2017-03-27 Thread Simon Riggs
On 25 March 2017 at 16:30, Dave Page wrote: > I believe this and other reasons we've described are exactly why other DBMS' > do what we're proposing. It would help review if you could show some links and give a commentary on what you think others do, what they get right and

Re: [HACKERS] Monitoring roles patch

2017-03-25 Thread Dave Page
Hi > On 25 Mar 2017, at 15:55, Stephen Frost wrote: > > Robert, > > * Robert Haas (robertmh...@gmail.com) wrote: >>> On Fri, Mar 24, 2017 at 12:46 PM, Dave Page wrote: On Fri, Mar 24, 2017 at 4:24 PM, Robert Haas wrote:

Re: [HACKERS] Monitoring roles patch

2017-03-25 Thread Stephen Frost
Robert, * Robert Haas (robertmh...@gmail.com) wrote: > On Fri, Mar 24, 2017 at 8:30 AM, Stephen Frost 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. I wasn't very clear with

Re: [HACKERS] Monitoring roles patch

2017-03-25 Thread Stephen Frost
Robert, * Robert Haas (robertmh...@gmail.com) wrote: > On Fri, Mar 24, 2017 at 12:46 PM, Dave Page wrote: > > On Fri, Mar 24, 2017 at 4:24 PM, Robert Haas wrote: > >> That's possible, but it really depends on the tool, not on core > >> PostgreSQL. The

Re: [HACKERS] Monitoring roles patch

2017-03-24 Thread Robert Haas
On Fri, Mar 24, 2017 at 12:46 PM, Dave Page wrote: > On Fri, Mar 24, 2017 at 4:24 PM, Robert Haas 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 >>>

Re: [HACKERS] Monitoring roles patch

2017-03-24 Thread Dave Page
On Fri, Mar 24, 2017 at 4:24 PM, Robert Haas 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. > >

Re: [HACKERS] Monitoring roles patch

2017-03-24 Thread Robert Haas
On Fri, Mar 24, 2017 at 8:30 AM, Stephen Frost 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

Re: [HACKERS] Monitoring roles patch

2017-03-24 Thread Stephen Frost
Peter, * Peter Eisentraut (peter.eisentr...@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

Re: [HACKERS] Monitoring roles patch

2017-03-23 Thread Peter Eisentraut
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

Re: [HACKERS] Monitoring roles patch

2017-03-23 Thread Dave Page
Hi On Thu, Mar 23, 2017 at 12:23 PM, Stephen Frost 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 >>

Re: [HACKERS] Monitoring roles patch

2017-03-23 Thread Stephen Frost
Dave, * Dave Page (dp...@pgadmin.org) wrote: > On Wed, Mar 22, 2017 at 3:46 PM, Dave Page wrote: > > On Wed, Mar 22, 2017 at 1:15 PM, Stephen Frost wrote: > >> I did specifically ask for explicit roles to be made to enable such > >> capability and that the

Re: [HACKERS] Monitoring roles patch

2017-03-23 Thread Dave Page
On Wed, Mar 22, 2017 at 3:46 PM, Dave Page wrote: > On Wed, Mar 22, 2017 at 1:15 PM, Stephen Frost 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 >>

Re: [HACKERS] Monitoring roles patch

2017-03-22 Thread Dave Page
On Wed, Mar 22, 2017 at 1:15 PM, Stephen Frost 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

Re: [HACKERS] Monitoring roles patch

2017-03-22 Thread Dave Page
On Wed, Mar 22, 2017 at 12:55 PM, Peter Eisentraut 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

Re: [HACKERS] Monitoring roles patch

2017-03-22 Thread Stephen Frost
Peter, * Peter Eisentraut (peter.eisentr...@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

Re: [HACKERS] Monitoring roles patch

2017-03-22 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: > On Wed, Mar 22, 2017 at 7:48 AM, Dave Page 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. > > >

Re: [HACKERS] Monitoring roles patch

2017-03-22 Thread Peter Eisentraut
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.

Re: [HACKERS] Monitoring roles patch

2017-03-22 Thread Robert Haas
On Wed, Mar 22, 2017 at 7:48 AM, Dave Page 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

Re: [HACKERS] Monitoring roles patch

2017-03-22 Thread Dave Page
On Wed, Mar 22, 2017 at 11:32 AM, Robert Haas wrote: > On Fri, Feb 24, 2017 at 5:14 AM, Dave Page 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() >>

Re: [HACKERS] Monitoring roles patch

2017-03-22 Thread Robert Haas
On Fri, Feb 24, 2017 at 5:14 AM, Dave Page 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() >

Re: [HACKERS] Monitoring roles patch

2017-03-21 Thread Peter Eisentraut
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

Re: [HACKERS] Monitoring roles patch

2017-03-21 Thread Denish Patel
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

Re: [HACKERS] Monitoring roles patch

2017-03-17 Thread Dave Page
Hi On Thu, Mar 16, 2017 at 7:04 PM, Denish Patel 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 >

Re: [HACKERS] Monitoring roles patch

2017-03-16 Thread Denish Patel
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

[HACKERS] Monitoring roles patch

2017-02-24 Thread Dave Page
Per the discussion at https://www.postgresql.org/message-id/CA%2BOCxoyYxO%2BJmzv2Micj4uAaQdAi6nq0w25BPQgLLxsrvTmREw%40mail.gmail.com, attached is a patch that implements the following: - Adds a default role called pg_monitor - Gives members of the pg_monitor role full access to: