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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Simon Riggs <simon(at)2ndQuadrant(dot)com>
Cc: Robert Haas <rhaas(at)postgresql(dot)org>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
Date: 2010-07-09 17:21:54
Message-ID: 18429.1278696114@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> On Fri, 2010-07-09 at 11:09 -0400, Tom Lane wrote:
>> Simon Riggs <simon(at)2ndQuadrant(dot)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

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Stephen Frost 2010-07-09 17:28:26 Re: [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
Previous Message Stephen Frost 2010-07-09 17:19:31 Re: [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2010-07-09 17:28:26 Re: [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
Previous Message Stephen Frost 2010-07-09 17:19:31 Re: [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().