Re: Proposal: SET ROLE hook

On 03/01/2016 02:09 AM, Pavel Stehule wrote:
> 2016-02-29 2:40 GMT+01:00 Joe Conway <mail(at)joeconway(dot)com
> <mailto:mail(at)joeconway(dot)com>>:
>
> On 01/07/2016 09:08 AM, Joe Conway wrote:
> > On 01/06/2016 10:36 AM, Tom Lane wrote:
> >> I think a design that was actually somewhat robust would require two
> >> hooks, one at check_role and one at assign_role, wherein the first one
> >> would do any potentially-failing work and package all required info into
> >> a blob that could be passed through to the assign hook.

> I see following issues:
>
> 1. Missing the possibility to pass custom data from SetRoleCheck_hook to
> SetRoleAssign_hook. Tom mentioned it in his comment.

You can pass the data between the two plugin hook functions in your
extension itself, so I see no need to try to pass custom data through
the backend. Do any of the other hooks even do that?

I think the main point was to have two hooks. The potentially-failing
work could be done during the check_role() hook and the collected info
could be used during the assign_role() hook. This works fine with the
patch as-is.

> 2. Missing little bit more comments and an explanation why and when to
> use these hooks.

Doesn't look all that different from existing user hooks to me, but
sure, I'll add a bit more to the comments.

The thing I wish we had was a place in the docs where we list all the
user plugin hooks. But as far as I know that doesn't exist (please
correct me if I'm wrong) and I am not volunteering to create it just for
the sake of this patch ;-)

Thanks for the review!

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development