On Thu, Mar 24, 2022 at 1:10 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > However, it might. And if it does, I think it would be best if > > removing that exception were the *only* change in this area made by > > that release. > > Good idea, especially since it's getting to be too late to consider > anything more invasive anyway.
I'd say it's definitely too late at this point. > > So I propose to commit something like what I posted here: > > http://postgr.es/m/ca+tgmobgek0jraowqvpqhsxcfbdfitxsomoebhmmmhmj4gl...@mail.gmail.com > > +1, although the comments might need some more work. In particular, > I'm not sure that this bit is well stated: > > + * A role cannot have WITH ADMIN OPTION on itself, because that would > + * imply a membership loop. > > We already do consider a role to be a member of itself: > > regression=# create role r; > CREATE ROLE > regression=# grant r to r; > ERROR: role "r" is a member of role "r" > regression=# grant r to r with admin option; > ERROR: role "r" is a member of role "r" > > It might be better to just say "By policy, a role cannot have WITH ADMIN > OPTION on itself". But if you want to write a defense of that policy, > this isn't a very good one. That sentence is present in the current code, along with a bunch of other sentences, which the patch renders irrelevant. So I just deleted all of the other stuff and kept the sentence that is still relevant to the revised code. I think your proposed replacement is an improvement, but let's be careful not to get sucked into too much of a wordsmithing exercise in a patch that's here to make a functional change. -- Robert Haas EDB: http://www.enterprisedb.com