po 14. 9. 2020 v 16:12 odesÃlatel Konstantin Knizhnik < k.knizh...@postgrespro.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 > >
