Re: [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms(). - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
Date
Msg-id 18429.1278696114@sss.pgh.pa.us
Whole thread Raw
In response to Re: [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().  (Simon Riggs <simon@2ndQuadrant.com>)
Responses Re: [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
Re: [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
Re: [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
List pgsql-hackers
Simon Riggs <simon@2ndQuadrant.com> writes:
> On Fri, 2010-07-09 at 11:09 -0400, Tom Lane wrote:
>> Simon Riggs <simon@2ndQuadrant.com> writes:
>>> Strangely, I was looking into removing the ExecCheckRTPerms check
>>> altogether by forcing plan invalidation when permissions are updated.
>>> That would be a performance tweak that would render this change useless.
>> 
>> That seems both pointless and wrong.  Permissions checks should happen
>> at execution time not plan time.

> 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.

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.

> 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.
        regards, tom lane


pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: Re: [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
Next
From: Simon Riggs
Date:
Subject: Re: ALTER TABLE SET STATISTICS requires AccessExclusiveLock