Re: [v9.2] Fix leaky-view problem, part 1

From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Noah Misch <noah(at)2ndquadrant(dot)com>
Cc: Kohei Kaigai <Kohei(dot)Kaigai(at)emea(dot)nec(dot)com>, Robert Haas <robert(dot)haas(at)enterprisedb(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [v9.2] Fix leaky-view problem, part 1
Date: 2011-07-02 17:54:28
Message-ID: CADyhKSVEL3pj_k8Y6HOCvCQObYOyRufhqDdyydjkOA2+2BzXVw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

BTW, regarding to the statement support for security barrier views,
the following syntax might be more consistent with existing ones:
CREATE VIEW view_name WITH ( param [=value]) AS query ... ;
rather than
CREATE SECURITY VIEW view_name AS query ...;

Any comments?

2011/7/2 Noah Misch <noah(at)2ndquadrant(dot)com>:
> On Sat, Jul 02, 2011 at 12:48:32PM +0200, Kohei KaiGai wrote:
>> > Let's see. ?Every qual list will have some depth d such that all quals having
>> > depth >= d are security-relevant, and all others are not security-relevant.
>> > (This does not hold for all means of identifying security-relevant quals, but
>> > it does hold for the CREATE SECURITY VIEW/reloptions strategy proposed in your
>> > part 2 patch.) ?Suppose you track whether each Query node represents a
>> > security view, then only increment the qualifier depth for such Query nodes,
>> > rather than all Query nodes. ?The tracked depth then becomes a security
>> > partition depth. ?Keep the actual sorting algorithm the same. ?(Disclaimer: I
>> > haven't been thinking about this nearly as long as you have, so I may be
>> > missing something relatively obvious.)
>> >
>> It might be an idea to increment the depth only when we go across security
>> barrier view. In other words, all the qualifiers will have same depth unless
>> it does not come from inside of the security view.
>
> Yes; that sounds suitable.
>
>> > As it stands, the patch badly damages the performance of this example:
>> >
>> > CREATE FUNCTION expensive(int) RETURNS boolean LANGUAGE sql
>> > ? ? ? ?AS 'SELECT pg_sleep(1); SELECT true' COST 1000000;
>> > CREATE TABLE t(c) AS SELECT * FROM generate_series(1,3);
>> > EXPLAIN ANALYZE
>> > ? ? ? ?SELECT * FROM (SELECT * FROM t WHERE expensive(c)) t0 WHERE c = 2;
>> >
>> > That doesn't even use a view, let alone a security view. ?While I like the
>> > patch's current simplicity, we need to narrow its impact.
>> >
>> If we apply above idea I explained, c=2 and expensive(c) will belong
>> to same depth,
>> then it shall be reordered according to cost estimation.
>> In the case when "(SELECT * FROM t WHERE expensive(c))" come from security
>> view, the performance damage is unavoidable, because DBA explicitly specified
>> its main purpose is security.
>>
>> So, it might be a good idea to split out my two patches into three.
>> 1. Add "SECURITY VIEW" support.
>> 2. Fix leaky view part.1 - order of qualifiers
>> 3. Fix leaky view part.2 - unexpected pushing down
>>
>> How about your opinion?
>
> I'd say, for CommitFest purposes, keep SECURITY VIEW attached to one of the
> other patches.  It's not likely it would be committed without anything hooked up
> to actually use it.  Splitting it out into its own patch *file* and attaching
> that and the part 1 patch to the same email would be fine, though.
>

--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2011-07-02 18:07:24 Re: Optimizing pg_trgm makesign() (was Re: WIP: Fast GiST index build)
Previous Message Tom Lane 2011-07-02 16:03:23 Re: clean.pl on Windows fails to remove flex output