Re: [PATCH] A hook for session start

On Tue, Nov 7, 2017 at 1:15 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:
>
> On Sun, Nov 5, 2017 at 3:14 AM, Fabrízio de Royes Mello
> <fabriziomello@gmail.com> wrote:
> > On Sat, Nov 4, 2017 at 1:23 AM, Michael Paquier <
michael.paquier@gmail.com>
> > wrote:
> >> On Fri, Nov 3, 2017 at 1:55 PM, Fabrízio de Royes Mello
> >> <fabriziomello@gmail.com> wrote:
> >> >> Passing the database name and user name does not look much useful to
> >> >> me. You can have access to this data already with CurrentUserId and
> >> >> MyDatabaseId.
> >> >
> >> > This way we don't need to convert oid to names inside hook code.
> >>
> >> Well, arguments of hooks are useful if they are used. Now if I look at
> >> the two examples mainly proposed in this thread, be it in your set of
> >> patches or the possibility to do some SQL transaction, I see nothing
> >> using them. So I'd vote for keeping an interface minimal.
> >>
> >
> > Maybe the attached patch with improved test module can illustrate
better the
> > feature.
>
> I was going to to hack something like that. That's interesting for the
> use case Robert has mentioned.
>
> Well, in the case of the end session hook, those variables are passed
> to the hook by being directly taken from the context from MyProcPort:
> +               (*session_end_hook) (MyProcPort->database_name,
> MyProcPort->user_name);
> In the case of the start hook, those are directly taken from the
> command outer caller, but similarly MyProcPort is already set, so
> those are strings available (your patch does so in the end session
> hook)... Variables in hooks are useful if those are not available
> within the memory context, and refer to a specific state where the
> hook is called. For example, take the password hook, this uses the
> user name and the password because those values are not available
> within the session context. The same stands for other hooks as well.
> Keeping the interface minimal helps in readability and maintenance.
> See for the attached example that can be applied on top of 0003, which
> makes use of the session context, the set regression tests does not
> pass but this shows how I think those hooks had better be shaped.
>

Makes sense... fixed.


> +       (*session_end_hook) (MyProcPort->database_name,
MyProcPort->user_name);
> +
> +       /* Make don't leave any active transactions and/or locks behind */
> +       AbortOutOfAnyTransaction();
> +       LockReleaseAll(USER_LOCKMETHOD, true);
> Let's leave this work to people actually implementing the hook contents.
>

Fixed.

Attached new version. I unify all three patches in just one because this is
a small patch and it's most part is test code.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello