Re: On login trigger: take three

On Mon, Oct 9, 2023 at 10:11 AM Alexander Korotkov <aekorotkov(at)gmail(dot)com> wrote:
> * Hold lock during setting of pg_database.dathasloginevt flag (v32
> version actually didn't prevent race condition).

So ... how does getting this flag set actually work? And how does
clearing it work?

In the case of row-level security, you have to explicitly enable the
flag on the table level using DDL provided for that purpose. In the
case of relhas{rules,triggers,subclass} the flag is set automatically
as a side-effect of some other operation. I tend to consider that the
latter design is somewhat messy. It's potentially even messier here,
because at least when you add a rule or a trigger to a table you're
expecting to take a lock on the table anyway. I don't think you
necessarily expect creating a login trigger to take a lock on the
database. That's a bit odd and could have visible side effects. And if
you don't, then what happens is that if you create two login triggers
in overlapping transactions, then (1) if there were no login triggers
previously, one of the transactions fails with an internal-looking
error message about a concurrent tuple update and (2) if there were
login triggers previously, then it works fine. That's also a bit weird
and surprising. Now the counter-argument could be that adding new DDL
to enable login triggers for a database is too much cognitive burden
and it's better to have the kind of weird and surprising behavior that
I just discussed. I don't know that I would buy that argument, but it
could be made ... and my real point here is that I don't even see
these trade-offs being discussed. Apologies if they were discussed
earlier and I just missed that; I confess to not having read every
email message on this topic, and some of the ones I did read I read a
long time ago.

> This version should be good and has no overhead. Any thoughts?
> Daniel, could you please re-run the performance tests?

Is "no overhead" an overly bold claim here?

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