Noah, * Noah Misch (n...@leadboat.com) wrote: > > Updated patch attached. I'll give it another good look and then commit > > it, barring objections. > > This thread and its satellite[1] have worked their way through a few designs. > At first, it was adding role attributes, alongside existing attributes like > REPLICATION and BYPASSRLS. It switched[2] to making pg_dump preserve ACLs on > system objects. Built-in roles joined[3] the pg_dump work to offer predefined > collections of ACL grants. Finally, it dropped[4] the pg_dump side and > hard-coded the roles into the features they govern.
Correct, after quite a bit of discussion and the conclusion that, while pg_dump support for dumping ACLs might be interesting, it was quite a bit more complex an approach than this use-case justified. Further, adding support to pg_dump for dumping ACLs could be done independently of default roles. The one argument which you've put forth for adding the complexity of dumping catalog ACLs is that we might reduce the number of default roles provided to the user. I disagree that we would. Having a single set of default roles which provide a sensible breakdown of permissions is a better approach than asking every administrator and application developer who is building tools on top of PG to try and work through what makes sense themselves, even if that means we have a default role with a small, or even only an individual, capability. I also disagree that we won't be able to adjust the privileges granted to a role in the future. We have certainly made adjustments to what a 'replication' role is able to do, which has largely been in the 'more capabilities' direction that you opine concern over. > To summarize, I think the right next step is to resume designing pg_dump > support for system object ACLs. I looked over your other two patches and will > unshelve those reviews when their time comes. To be clear, I don't believe the two patches are particularly involved with each other and don't feel that one needs to wait for the other. Further, I'm not convinced that adding support for dumping ACLs or, in general, encouraging users to define their own ACLs on catalog objects is a good idea. We certainly have no mechanism in place today for those ACLs to be respected by SysCache and encouraging their use when we won't actually respect them is likely to be confusing. I had thought differently at one point but my position changed during the discussion when I realized the complexity and potential confusion it could cause and considered that against the simplicity and relatively low cost of having default roles. Thanks! Stephen
signature.asc
Description: Digital signature
