Re: Additional role attributes && superuser review

On Wed, Apr 29, 2015 at 10:47 AM, Stephen Frost <sfrost(at)snowman(dot)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