Re: On login trigger: take three

Hi, Robert!

On Tue, Oct 3, 2023 at 5:21 PM Robert Haas <robertmhaas@gmail.com> wrote:
> On Tue, Oct 3, 2023 at 9:43 AM Daniel Gustafsson <daniel@yesql.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.

Thank you for the interesting ideas. I'd like to try to revive the
version with the flag in pg_database.  Will use other ideas as backup
if no success.

------
Regards,
Alexander Korotkov