Stephen, 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. Sounds good. I'll start on those changes next. Thanks, Adam -- Adam Brightwell - adam.brightw...@crunchydatasolutions.com Database Engineer - www.crunchydatasolutions.com
