The comment above is pretty big and I don't think we want to completely remove it. Can you add it as another 'Note' in the 'has_role_attribute' comment and reword it accordingly?
Ok, agreed. Will address.
Whitespace issue that should be fixed- attributes should line up with roleid.
Ok. Will address.
It occurs to me that in this case (and a few others..), we're doing a lot of extra work- each call to pg_check_role_attribute() is doing a lookup on the oid just to get back to what the rolattr value on this row is. How about a function which takes rolattr and the text representation of the attribute and returns if the bit is set for that rolattr value? Then you'd pass pg_authid.rolattr into the function calls above instead of the role's oid.
Makes sense, I'll put that together.
I don't see any changes to the regression test files, were they forgotten in the patch? I would think that at least the view definition changes would require updates to the regression tests, though perhaps nothing else.
Hmmm... :-/ The regression tests that changed were in 'src/test/regress/expected/rules.out' and should be near the bottom of the patch.
Overall, I'm pretty happy with the patch and would suggest moving on to writing up the documentation changes to go along with the code changes. I'll continue to play around with it but it all seems pretty clean to me and will allow us to easily add the additiaonl role attributes being discussed.
