Prev: [COMMITTERS] pgsql: Stamp HEAD as 9.1devel.
Next: [HACKERS] FYI: Ubuntu 10.04 lucid strange segfault
From: Simon Riggs on 9 Jul 2010 14:17
On Fri, 2010-07-09 at 14:01 -0400, Tom Lane wrote:
> Simon Riggs <simon(a)2ndQuadrant.com> writes:
> > On Fri, 2010-07-09 at 13:21 -0400, Tom Lane wrote:
> >> Except that it *is* a change in behavior: the first check will occur
> >> too soon.
>
> > Sooner matters why?
>
> Consider PREPARE followed only later by EXECUTE. Your proposal would
> make the PREPARE fail outright, when it currently does not.

Just to avoid wasted investigation: are you saying that is important
behaviour that is essential we retain in PostgreSQL, or will you hear
evidence that supporting that leads to a performance decrease elsewhere?

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Training and Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Tom Lane on 9 Jul 2010 17:21
Simon Riggs <simon(a)2ndQuadrant.com> writes:
> On Fri, 2010-07-09 at 14:01 -0400, Tom Lane wrote:
>> Consider PREPARE followed only later by EXECUTE. Your proposal would
>> make the PREPARE fail outright, when it currently does not.

> Just to avoid wasted investigation: are you saying that is important
> behaviour that is essential we retain in PostgreSQL, or will you hear
> evidence that supporting that leads to a performance decrease elsewhere?

Well, I think that that problem makes moving the checks into the planner
a nonstarter. But as somebody pointed out upthread, you could still get
what you want by keeping a flag saying "permission checks have been
done" so the executor could skip the checks on executions after the
first. I'd still want to see some evidence showing that it's worth
troubling over though. Premature optimization being the root of all
evil, and all that. (In this case, the hazard we expose ourselves to
seems to be security holes due to missed resets of the flag.)

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Simon Riggs on 11 Jul 2010 03:58
On Fri, 2010-07-09 at 17:21 -0400, Tom Lane wrote:
> Simon Riggs <simon(a)2ndQuadrant.com> writes:
> > On Fri, 2010-07-09 at 14:01 -0400, Tom Lane wrote:
> >> Consider PREPARE followed only later by EXECUTE. Your proposal would
> >> make the PREPARE fail outright, when it currently does not.
>
> > Just to avoid wasted investigation: are you saying that is important
> > behaviour that is essential we retain in PostgreSQL, or will you hear
> > evidence that supporting that leads to a performance decrease elsewhere?
>
> Well, I think that that problem makes moving the checks into the planner
> a nonstarter. But as somebody pointed out upthread, you could still get
> what you want by keeping a flag saying "permission checks have been
> done" so the executor could skip the checks on executions after the
> first.

Seems like best idea.

> I'd still want to see some evidence showing that it's worth
> troubling over though. Premature optimization being the root of all
> evil, and all that. (In this case, the hazard we expose ourselves to
> seems to be security holes due to missed resets of the flag.)

If we did this it would be to add one line to the code

if (!perms_ok)

That doesn't seem to fall into the category of evil optimization to me.
I've seen you recode other parts of the executor stating reasons like
"shave another few cycles off the main path" and that seems the case
here. We shouldn't need to debate the consequences of Amhdahls law each
time.


Attached is a script to allow pgbench to be used to measure difference
between superuser and a typical privilege model for the pgbench tables.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Training and Services
From: Tom Lane on 11 Jul 2010 11:44
Simon Riggs <simon(a)2ndQuadrant.com> writes:
> On Fri, 2010-07-09 at 17:21 -0400, Tom Lane wrote:
>> I'd still want to see some evidence showing that it's worth
>> troubling over though. Premature optimization being the root of all
>> evil, and all that. (In this case, the hazard we expose ourselves to
>> seems to be security holes due to missed resets of the flag.)

> If we did this it would be to add one line to the code
> if (!perms_ok)

> That doesn't seem to fall into the category of evil optimization to me.

The problem I foresee is not in the testing of the flag, it's in the
setting/resetting of it. It's a reliability penalty not a performance
penalty --- and any mistakes would count as security issues.

Now it may be that you can offer a convincing argument that no such
mistake/oversight is likely. But you haven't even tried to make that
case. Even if you can show that the risk is small, it's not going to
be zero, so we have to trade it off against a demonstrated performance
improvement.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Robert Haas on 11 Jul 2010 12:11
On Jul 11, 2010, at 10:44 AM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote:
> Simon Riggs <simon(a)2ndQuadrant.com> writes:
>> On Fri, 2010-07-09 at 17:21 -0400, Tom Lane wrote:
>>> I'd still want to see some evidence showing that it's worth
>>> troubling over though. Premature optimization being the root of all
>>> evil, and all that. (In this case, the hazard we expose ourselves to
>>> seems to be security holes due to missed resets of the flag.)
>
>> If we did this it would be to add one line to the code
>> if (!perms_ok)
>
>> That doesn't seem to fall into the category of evil optimization to me.
>
> The problem I foresee is not in the testing of the flag, it's in the
> setting/resetting of it. It's a reliability penalty not a performance
> penalty --- and any mistakes would count as security issues.
>
> Now it may be that you can offer a convincing argument that no such
> mistake/oversight is likely. But you haven't even tried to make that
> case. Even if you can show that the risk is small, it's not going to
> be zero, so we have to trade it off against a demonstrated performance
> improvement.

There's no point in going back and forth here until we have a patch and the results of some performance testing using said patch. If Simon writes one and submits it with some results, we'll consider it on the merits. I think that's all Simon is asking for, and I don't think anyone is seriously arguing anything to the contrary. Like Tom, I'm skeptical that there is much performance to be found here, but if I'm wrong, I'm happy to have someone demonstrate it.

....Robert
--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

First  |  Prev  |  Next  |  Last
Pages: 1 2 3 4 5
Prev: [COMMITTERS] pgsql: Stamp HEAD as 9.1devel.
Next: [HACKERS] FYI: Ubuntu 10.04 lucid strange segfault