I still don't see any point in trying to pass data back from the hooks as the extension can maintain that state just fine, although it looks like it would be pretty trivial to do using a new void* member added to role_auth_extra. Tom (or anyone else), any comment on that?
I think it's possibly worth doing, but since auth is simple and linear I'm not overly bothered. It'd hardly be the first place Pg relied on passing information between functions using globals.
If you expect to daisy-chain hooks then a private-data member isn't too helpful, since a prior hook in the call chain may have set it already and you don't know what's there or how to manage it. So overall, I'm -1 for a private_data arg, I don't think it gains us anything in lifetime management, code clarity etc and it makes daisy-chaining way more complex.
I don't see what the point of SetRoleAssign_hook is, since it returns void and doesn't have out parameters. If it's expected to takes some action, what is it? If it's meant to modify myextra->roleid and myextra->is_superuser prior to their use by SetCurrentRoleId they should be passed pointer-indirected so they can be modified by the hook.
I'd like to see parameter names specified in the hook type definition.
It needs tests added, along with a note somewhere on usage of the hook that mentions the usual pattern for using it, possibly in the test/example. Something like:
Also behaviour of each hook should be more completely specified. Can it elog(ERROR)? What happens if it does? What can it safely change and what not? Are there restrictions to what it can do in terms of access to syscache/relcache/heap etc?
Why do you pass GetSessionUserId() to the hook given that the caller can look it up directly? Just a convenience?
