On Wed, Apr 29, 2015 at 10:47 AM, Stephen Frost <sfr...@snowman.net> wrote: > Here is the latest revision of this patch.
I think this patch is too big and does too many things. It should be broken up into small patches which can be discussed and validated independently. The fact that your commit message is incredibly long is a sign that there's too much going on here, and that message doesn't even cover all of it. It seems to me that the core change here is really to pg_dump. You're adding the ability for pg_dump to dump and restore privileges on objects in pg_dump. That capability is a complete - and useful - feature in its own right, with no other changes. Then the next thing you've got here is a series of patches that change the rights to execute various utility functions. Some of those, like the changes to pg_stop_backup(), are pretty much a slam-dunk, because they don't really change user-visible behavior; they just give the DBA more options. Others, like the changes to replication permissions, are likely to be more controversial. You should split the stuff that's a slam-dunk apart from the stuff that makes policy decisions, and plan to commit the former changes first, and the latter changes only if and when there is very clear agreement on the specific policies to be changed. Finally, you've got the idea of making pg_ a reserved prefix for roles, adding some predefined roles, and giving them some predefined privileges. That should be yet another patch. I think that if you commit this the way you have it today, everybody will go, oh, look, Stephen committed something, but it looks complicated, I won't pay attention. And then, six months from now when we're in beta, or maybe after final, people will start looking at this and realizing that there are parts of it they hate, but it will be hard to fix at that point. Breaking it out will hopefully allow more discussion on the individual features of in here, of which there are probably at least four. It will also make it easier to revert part of it rather than all of it if that turns out to be needed. -- 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
