On login trigger: take three

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

On login trigger: take three

konstantin knizhnik
Hi hackers,

Recently I have asked once again by one of our customers about login
trigger in postgres. People are migrating to Postgres from Oracle and 
looking for Postgres analog of this Oracle feature.
This topic is not new:

https://www.postgresql.org/message-id/flat/1570308356720-0.post%40n3.nabble.com#4748bcb0c5fc98cec0a735dbdffb0c68
https://www.postgresql.org/message-id/flat/OSAPR01MB507373499CCCEA00EAE79875FE2D0%40OSAPR01MB5073.jpnprd01.prod.outlook.com#ed50c248be32be6955c385ca67d6cdc1

end even session connect/disconnect hooks were sometimes committed (but
then reverted).
As far as I understand most of the concerns were related with disconnect
hook.
Performing some action on session disconnect is actually much more
complicated than on login.
But customers are not needed it, unlike actions performed at session start.

I wonder if we are really going to make some steps in this directions?
The discussion above was finished with "We haven't rejected the concept
altogether, AFAICT"

I have tried to resurrect this patch and implement on-connect trigger on
top of it.
The syntax is almost the same as proposed by Takayuki:

CREATE EVENT TRIGGER mytrigger
AFTER CONNECTION ON mydatabase
EXECUTE {PROCEDURE | FUNCTION} myproc();

I have replaced CONNECT with CONNECTION because last keyword is already
recognized by Postgres and
make ON clause mandatory to avoid shift-reduce conflicts.

Actually specifying database name is redundant, because we can define
on-connect trigger only for self database (just because triggers and
functions are local to the database).
It may be considered as argument against handling session start using
trigger. But it seems to be the most natural mechanism for users.

On connect trigger can be dropped almost in the same way as normal (on
relation) trigger, but with specifying name of the database instead of
relation name:

DROP TRIGGER mytrigger ON mydatabase;

It is possible to define arbitrary number of on-connect triggers with
different names.

I attached my prototype implementation of this feature.
I just to be sure first that this feature will be interested to community.
If so, I will continue work in it and prepare new version of the patch
for the commitfest.

Example

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


on_login_trigger.patch (35K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: On login trigger: take three

Pavel Stehule
Hi

čt 3. 9. 2020 v 15:43 odesílatel Konstantin Knizhnik <[hidden email]> napsal:
Hi hackers,

Recently I have asked once again by one of our customers about login
trigger in postgres. People are migrating to Postgres from Oracle and 
looking for Postgres analog of this Oracle feature.
This topic is not new:

https://www.postgresql.org/message-id/flat/1570308356720-0.post%40n3.nabble.com#4748bcb0c5fc98cec0a735dbdffb0c68
https://www.postgresql.org/message-id/flat/OSAPR01MB507373499CCCEA00EAE79875FE2D0%40OSAPR01MB5073.jpnprd01.prod.outlook.com#ed50c248be32be6955c385ca67d6cdc1

end even session connect/disconnect hooks were sometimes committed (but
then reverted).
As far as I understand most of the concerns were related with disconnect
hook.
Performing some action on session disconnect is actually much more
complicated than on login.
But customers are not needed it, unlike actions performed at session start.

I wonder if we are really going to make some steps in this directions?
The discussion above was finished with "We haven't rejected the concept
altogether, AFAICT"

I have tried to resurrect this patch and implement on-connect trigger on
top of it.
The syntax is almost the same as proposed by Takayuki:

CREATE EVENT TRIGGER mytrigger
AFTER CONNECTION ON mydatabase
EXECUTE {PROCEDURE | FUNCTION} myproc();

I have replaced CONNECT with CONNECTION because last keyword is already
recognized by Postgres and
make ON clause mandatory to avoid shift-reduce conflicts.

Actually specifying database name is redundant, because we can define
on-connect trigger only for self database (just because triggers and
functions are local to the database).
It may be considered as argument against handling session start using
trigger. But it seems to be the most natural mechanism for users.

On connect trigger can be dropped almost in the same way as normal (on
relation) trigger, but with specifying name of the database instead of
relation name:

DROP TRIGGER mytrigger ON mydatabase;

It is possible to define arbitrary number of on-connect triggers with
different names.

I attached my prototype implementation of this feature.
I just to be sure first that this feature will be interested to community.
If so, I will continue work in it and prepare new version of the patch
for the commitfest.


I have a customer that requires this feature too. Now it uses a solution based on dll session autoloading.  Native solution can be great.

+1

Pavel

Example

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

Reply | Threaded
Open this post in threaded view
|

RE: On login trigger: take three

tsunakawa.takay@fujitsu.com
In reply to this post by konstantin knizhnik
From: Konstantin Knizhnik <[hidden email]>
> Recently I have asked once again by one of our customers about login trigger in
> postgres. People are migrating to Postgres from Oracle and looking for Postgres
> analog of this Oracle feature.
> This topic is not new:

> I attached my prototype implementation of this feature.
> I just to be sure first that this feature will be interested to community.
> If so, I will continue work in it and prepare new version of the patch for the
> commitfest.

Thanks a lot for taking on this! +10

> It may be considered as argument against handling session start using trigger.
> But it seems to be the most natural mechanism for users.

Yeah, it's natural, just like the Unix shells run some shell scripts in the home directory.


Regards
Takayuki Tsunakawa

Reply | Threaded
Open this post in threaded view
|

Re: On login trigger: take three

konstantin knizhnik
Sorry, attached version of the patch is missing changes in one file.
Here is it correct patch.


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


on_login_trigger-2.patch (35K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: On login trigger: take three

konstantin knizhnik
In reply to this post by Pavel Stehule


On 03.09.2020 17:18, Pavel Stehule wrote:
Hi

čt 3. 9. 2020 v 15:43 odesílatel Konstantin Knizhnik <[hidden email]> napsal:
Hi hackers,

Recently I have asked once again by one of our customers about login
trigger in postgres. People are migrating to Postgres from Oracle and 
looking for Postgres analog of this Oracle feature.
This topic is not new:

https://www.postgresql.org/message-id/flat/1570308356720-0.post%40n3.nabble.com#4748bcb0c5fc98cec0a735dbdffb0c68
https://www.postgresql.org/message-id/flat/OSAPR01MB507373499CCCEA00EAE79875FE2D0%40OSAPR01MB5073.jpnprd01.prod.outlook.com#ed50c248be32be6955c385ca67d6cdc1

end even session connect/disconnect hooks were sometimes committed (but
then reverted).
As far as I understand most of the concerns were related with disconnect
hook.
Performing some action on session disconnect is actually much more
complicated than on login.
But customers are not needed it, unlike actions performed at session start.

I wonder if we are really going to make some steps in this directions?
The discussion above was finished with "We haven't rejected the concept
altogether, AFAICT"

I have tried to resurrect this patch and implement on-connect trigger on
top of it.
The syntax is almost the same as proposed by Takayuki:

CREATE EVENT TRIGGER mytrigger
AFTER CONNECTION ON mydatabase
EXECUTE {PROCEDURE | FUNCTION} myproc();

I have replaced CONNECT with CONNECTION because last keyword is already
recognized by Postgres and
make ON clause mandatory to avoid shift-reduce conflicts.

Actually specifying database name is redundant, because we can define
on-connect trigger only for self database (just because triggers and
functions are local to the database).
It may be considered as argument against handling session start using
trigger. But it seems to be the most natural mechanism for users.

On connect trigger can be dropped almost in the same way as normal (on
relation) trigger, but with specifying name of the database instead of
relation name:

DROP TRIGGER mytrigger ON mydatabase;

It is possible to define arbitrary number of on-connect triggers with
different names.

I attached my prototype implementation of this feature.
I just to be sure first that this feature will be interested to community.
If so, I will continue work in it and prepare new version of the patch
for the commitfest.


I have a customer that requires this feature too. Now it uses a solution based on dll session autoloading.  Native solution can be great.

+1

I realized that on connect trigger should be implemented as EVENT TRIGGER.
So I have reimplemented my patch using event trigger and use session_start even name to make it more consistent with other events.
Now on login triggers can be created in this way:

create table connects(id serial, who text);
create function on_login_proc() returns event_trigger as $$
begin
  insert into connects (who) values (current_user());
  raise notice 'You are welcome!';
end;
$$ language plpgsql;
create event trigger on_login_trigger on session_start execute procedure on_login_proc();
alter event trigger on_login_trigger enable always;




on_connect_event_trigger-6.patch (15K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: On login trigger: take three

Pavel Stehule
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



Reply | Threaded
Open this post in threaded view
|

Re: On login trigger: take three

konstantin knizhnik


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.

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?



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


on_connect_event_trigger-7.patch (17K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: On login trigger: take three

Pavel Stehule


po 14. 9. 2020 v 16:12 odesílatel Konstantin Knizhnik <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

Re: On login trigger: take three

konstantin knizhnik


On 14.09.2020 17:34, Pavel Stehule wrote:
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. 

I agree.
I can rename trigger to backend_start or process_start or whatever else.

 

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.

It is always possible to login by disabling startup triggers using disable_session_start_trigger GUC:

psql "dbname=postgres options='-c disable_session_start_trigger=true'"


Reply | Threaded
Open this post in threaded view
|

Re: On login trigger: take three

Pavel Stehule


po 14. 9. 2020 v 17:53 odesílatel Konstantin Knizhnik <[hidden email]> napsal:


On 14.09.2020 17:34, Pavel Stehule wrote:
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. 

I agree.
I can rename trigger to backend_start or process_start or whatever else.

Creating a good name can be hard - it is not called for any process - so maybe "user_backend_start" ? 


 

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.

It is always possible to login by disabling startup triggers using disable_session_start_trigger GUC:

psql "dbname=postgres options='-c disable_session_start_trigger=true'"

sure, I know. Just this behavior can be a very unpleasant surprise, and my question is if it can be fixed.  Creating custom libpq variables can be the stop for people that use pgAdmin.