Re: Wrong security context for deferred triggers?

Laurenz Albe <laurenz.albe@cybertec.at> writes:
> [ v4-0001-Make-AFTER-triggers-run-with-the-correct-user.patch ]

I started to look at this and got distracted by wondering why
afterTriggerAddEvent's scanning loop wasn't checking ats_modifiedcols.
That led to ea68ea632, which means this now needs a minor rebase.

I have a couple other thoughts:

* It's kind of sad that we have to add yet another field to
AfterTriggerSharedData, especially since this one will cost 8 bytes
thanks to alignment considerations.  I'm not sure if there's another
way, but it seems like ats_relid, ats_rolid, and ats_modifiedcols
are all going to be extremely redundant in typical scenarios.
Maybe it's worth rethinking that data structure a bit?  Or maybe
I'm worried over nothing; perhaps normal situations have only a few
AfterTriggerSharedDatas anyway.  It might be worth trying to gather
some statistics about that.

* I don't buy the bit about "We have to check if the role still
exists"; I think that's just a waste of cycles.  There is no check
that prevents dropping the role a session is currently running as,
so why do we need one here?  Moreover, the role could still get
dropped immediately after you look.  Or for that matter, save_rolid
could be invalid by the time you restore it.  None of this bothers me,
because if a session is running as a dropped role it will still
function pretty normally.  It will find that it has no privileges
beyond those granted to PUBLIC, and if it tries to inspect
current_user or related functions those will fail, but there's
no compromise to system integrity.  So I'd just remove that test.

			regards, tom lane