On Sun, Nov 5, 2017 at 3:14 AM, Fabrízio de Royes Mello <fabriziome...@gmail.com> wrote: > On Sat, Nov 4, 2017 at 1:23 AM, Michael Paquier <michael.paqu...@gmail.com> > wrote: >> On Fri, Nov 3, 2017 at 1:55 PM, Fabrízio de Royes Mello >> <fabriziome...@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. + (*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. -- Michael
session_hook_simplify.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
