Re: On login trigger: take three

On Tue, Oct 3, 2023 at 9:43 AM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
> That's exactly what happens, the patch is using BuildEventTriggerCache() to
> build the hash for EVT which is then checked for login triggers. This is
> clearly the bottleneck and there needs to be a fast-path. There used to be a
> cache flag in an earlier version of the patch but it was a but klugy, a version
> of that needs to be reimplemented for this patch to fly.

So I haven't looked at this patch, but we basically saying that only
the superuser can create login triggers, and if they do, those
triggers apply to every single user on the system? That would seem to
be the logical extension of the existing event trigger mechanism, but
it isn't obviously as good of a fit for this case as it is for other
cases where event triggers are a thing.

Changing the catalog representation could be a way around this. What
if you only allowed one login trigger per database, and instead of
being stored in pg_event_trigger, the OID of the function gets
recorded in the pg_database row? Then this would be a lot cheaper
since we have to fetch the pg_database row anyway. Or change the SQL
syntax to something entirely new so you can have different login
triggers for different users -- and maybe users are allowed to create
their own -- but the relevant ones can be found by an index scan
instead of a sequential scan.

I'm just spitballing here. If you think the present design is good and
just want to try to speed it up, I'm not deeply opposed to that. But
it's also not obvious to me how to stick a cache in front of something
that's basically a full-table scan.

--
Robert Haas
EDB: http://www.enterprisedb.com