Stephen Frost wrote: > Alvaro, > > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > > FWIW I've been giving this patch a look and and adjusting some coding > > details here and there. Do you intend to commit it yourself? You're > > not listed as reviewer or committer for it in the commitfest app, FWIW. > > Oh, great, thanks! And, yeah, I'm not as good as I should be about > updating the commitfest app. As for committing it, I was thinking I > would but you're certainly welcome to if you're interested. I would > like this to be committed soonish as it will then allow Adam to rebase > the patch which adds the various role attributes discussed previously on > top of the representation changes. I suspect he's done some work in > that direction already, but I keep asking for changes to this patch > which would then ripple down into the other.
Sure, I will go over it a bit more and merge changes from Adam to the docs as they come through, and commit soon. > > One thing I don't very much like is that check_role_attribute() receives > > a RoleAttr but nowhere it checks that only one bit is set in > > 'attribute'. From the coding, this routine would return true if just > > one of those bits is set, which is probably not what is intended. Now I > > realize that current callers all pass a bitmask with a single bit set, > > but I think it'd be better to have some protection against that, for > > possible future coding mistakes. > > That's certainly a good point. I'm inclined to suggest that there be an > Assert() check in check_role_attribute(), or were you thinking of > something else..? Yeah, an Assert() is what I had in mind. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
