Thanks for looking at this patch.
> I suggest moving the rest of the changes into separate patches. > Hmmm... perhaps the following? * superuser-cleanup - contains above mentioned superuser shortcuts only. * has_privilege-cleanup - contains has_*_priviledge cleanup only. Would that also require a separate commitfest entry? The ha*_something_privilege() changes are also not very consistent. > > We already have have_createrole_privilege(), which does include a > superuser check, and you add has_replication_privilege() with a > superuser check, but has_catupdate_privilege() and > has_inherit_privilege() don't include a superuser check. That's clearly > a mess. > Good catch. Though, according to the documentation, not even superuser is allowed to bypass CATUPDATE. http://www.postgresql.org/docs/9.4/static/catalog-pg-authid.html. However, I can't think of a reason why "inherit" wouldn't need the superuser check. Obviously superuser is considered a member of every role, but is there a reason that a superuser would not be allowed to bypass this? I only ask because it did not have a check previously, so I figure there might have been a good reason for it? Btw., why rename have_createrole_privilege()? > Well, actually it wasn't necessarily a rename. It was a removal of that function all together as all it did was simply return the result of "has_createrole_privilege". That seemed rather redundant and unnecessary, IMO. Also, your patch has spaces between tabs. Check for whitespace errors > with git. > Yikes. -Adam -- Adam Brightwell - adam.brightw...@crunchydatasolutions.com Database Engineer - www.crunchydatasolutions.com
