Re: [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> > Agreed that permission checks should logically be applied at execution
> > time. I am proposing a performance optimisation, not a change in
> > behaviour.
>
> Except that it *is* a change in behavior: the first check will occur too
> soon.

Yeah, I have to say that I don't see any way you could avoid the
behaviour change from doing this. Specifically, you can prepare plans
today that you don't have access to run and then run them later after
you've been granted the permission.

I'm not saying that's a huge use-case or anything, but moving the checks
to planner time would definitely change the behavior. No clue if any of
this is covered in the SQL spec.

> The fact that we're interested in adding plugin permissions checking
> pretty much destroys the idea anyway. You cannot assume that a plan
> cache invalidation will happen for any change in external state that
> a plugin might be consulting.

Yeah, this would certainly be a problem too, unless we kept the plugin
hook in the executor and only used the "optimization" for the stock PG
checks.

> > The proposed performance enhancement would be very useful since we have
> > to check permissions of functions, views, tables and every other aspect.
> > We could spend a while quantifying that overhead, though "non-zero" is
> > all we need to know.
>
> No, it's not all we need to know. If you can't prove the overhead
> involved here is significant, we should not be expending effort and
> creating subtle behavioral changes in pursuit of a minor optimization.

Agreed.

Thanks,

Stephen