Re: On login trigger: take three

> 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?
>
> I like this approach more than implementation of shared hash.
> 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
>
>