Re: On login trigger: take three - Mailing list pgsql-hackers
| From | Pavel Stehule |
|---|---|
| Subject | Re: On login trigger: take three |
| Date | |
| Msg-id | CAFj8pRBYwFTo8XOe=NV0hLoWeBykqwrUU0oKy-Z9vntseqJE5w@mail.gmail.com Whole thread Raw |
| In response to | Re: On login trigger: take three (Konstantin Knizhnik <k.knizhnik@postgrespro.ru>) |
| Responses |
Re: On login trigger: take three
|
| List | pgsql-hackers |
I like this approach more than implementation of shared hash.Please notice that we still need GUC to disable on-login triggers: to make it possible for superuser who did mistake and defined incorrect on-login trigger to
login to the system.
Do we need GUC to disable all other event triggers? May be I am wrong, but I do not see much need in such GUC: error in any of such event triggers is non fatal
and can be easily reverted.
So the only question is whether "disable_client_connection_trigger" should be true by default or not...
I agree with you that @2 is a little bit chaotic and @1 looks like a workaround.
But from my point of view @3 is not the best solution but overkill: maintaining yet another shared hash just to save few milliseconds on login seems to be too high price.
Actually there are many things which are loaded by new backend from the database on start: for example - catalog.
This is why launch of new backend is an expensive operation.
Certainly if we execute "select 1", then system catalog is not needed...
But does anybody start new backend just to execute "select 1" and exit?I understand so the implementation of a new shared cache can be a lot of work. The best way is enhancing pg_database about one column with information about the login triggers (dathaslogontriggers). In init time these data are in syscache, and can be easily checked. Some like pg_attribute have an atthasdef column. Same fields has pg_class - relhasrules, relhastriggers, ... Then the overhead of this design should be really zero.What do you think about it?
But still I have some concerns:
1. pg_database table format has to be changed. Certainly it is not something completely unacceptable. But IMHO we should try to avoid
modification of such commonly used catalog tables as much as possible.
yes, I know. Unfortunately I don't see any other and correct solution. There should be more wide discussion before this work about this topic. On second hand, this change should not break anything (if this new field will be placed on as the last field). The logon trigger really looks like a database trigger - so I think so this semantic is correct. I have no idea if it is acceptable for committers :-/. I hope so.
The fact that the existence of a logon trigger can be visible from a shared database object can be an interesting feature too.
2. It is not so easy to maintain this flag. There can be multiple on-login triggers defined. If such trigger is dropped, we can not just clear this flag.
We should check if other triggers exist. Now assume that there are two triggers and two concurrent transactions dropping each one.
According to their snapshots them do not see changes made by other transaction. So them remove both triggers but didn't clear the flag.
Certainly we can use counter instead of flag. But I am not sure that their will be not other problems with maintaining counter.
I don't think it is necessary. My opinion is not too strong, but if pg_class doesn't need to hold a number of triggers, then I think so pg_database doesn't need to hold this number too.
-- Konstantin Knizhnik Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
pgsql-hackers by date: