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

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.