Re: On login trigger: take three

po 14. 9. 2020 v 16:12 odesílatel Konstantin Knizhnik <
k(dot)knizhnik(at)postgrespro(dot)ru> napsal:

>
>
> On 14.09.2020 12:44, Pavel Stehule wrote:
> > Hi
> >
> > I am checking last patch, and there are notices
> >
> > 1. disable_session_start_trigger should be SU_BACKEND instead SUSET
> >
> > 2. The documentation should be enhanced - there is not any note about
> > behave when there are unhandled exceptions, about motivation for this
> > event trigger
> >
> > 3. regress tests should be enhanced - the cases with exceptions are
> > not tested
> >
> > 4. This trigger is not executed again after RESET ALL or DISCARD ALL -
> > it can be a problem if somebody wants to use this trigger for
> > initialisation of some session objects with some pooling solutions.
> >
> > 5. The handling errors don't work well for canceling. If event trigger
> > waits for some event, then cancel disallow connect although connected
> > user is superuser
> >
> > CREATE OR REPLACE FUNCTION on_login_proc2() RETURNS EVENT_TRIGGER AS
> > $$ begin perform pg_sleep(10000); raise notice '%', fx1(100);raise
> > notice 'kuku kuku'; end $$ language plpgsql;
> >
> > probably nobody will use pg_sleep in this routine, but there can be
> > wait on some locks ...
> >
> > Regards
> >
> > Pavel
> >
> >
> >
>
> Hi
> Thank you very much for looking at my patch for connection triggers.
> I have fixed 1-3 issues in the attached patch.
> Concerning 4 and 5 I have some doubts:
>
> 4. Should I add some extra GUC to allow firing of session_start trigger
> in case of RESET ALL or DISCARD ALL ?
> Looks like such behavior contradicts with event name "session_start"...
> And do we really need it? If some pooler is using RESET ALL/DISCARD ALL
> to emulate session semantic then most likely it provides way to define
> custom actions which
> should be perform for session initialization. As far as I know, for
> example pgbouncer allows do define own on-connect hooks.
>

If we introduce buildin session trigger , we should to define what is the
session. Your design is much more related to the process than to session.
So the correct name should be "process_start" trigger, or some should be
different. I think there are two different events - process_start, and
session_start, and there should be two different event triggers. Maybe the
name "session_start" is just ambiguous and should be used with a different
name.

>
> 5. I do not quite understand your concern. If I define trigger
> procedure which is blocked (for example as in your example), then I can
> use pg_cancel_backend to interrupt execution of login trigger and
> superuser can login. What should be changed here?
>

You cannot run pg_cancel_backend, because you cannot open a new session.
There is a cycle.

Regards

Pavel

>
>
>
> --
> Konstantin Knizhnik
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>
>