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

>> > I was referring to this paragraph:
>> >
>> > ?On the technical side, I am pretty doubtful that the approach of adding a
>> > ?nestlevel to FuncExpr and RelOptInfo is the right way to go. ?I believe we
>> > ?have existing code (to handle left joins) that prevents quals from being
>> > ?pushed down too far by fudging the set of relations that are supposedly needed
>> > ?to evaluate the qual. ?I suspect a similar approach would work here.
>> >
>> It seems to me the later half of this paragraph is talking about the problem of
>> unexpected qualifier pushing-down over the security barrier; I'm trying to solve
>> the problem with the part.2 patch.
>> The scenario the part.1 patch tries to solve is order to launch qualifiers, not
>> unexpected pushing-down.
>
> Okay, you're probably correct that it wasn't referring to the topic at hand.
> I'm still suspicious of the silent assumption about how quals can be assigned
> to plan nodes, but I don't have any concrete ideas for avoiding that.
>
If a subquery is enough simple to pull up, pull_up_simple_subquery() pulls up
the subquery. Its simpleness is checked by is_simple_subquery().
At that timing, qualifiers of the subquery are also pulled-up, so upper level
query shall have qualifiers with mixed nest-level.

Then, every qualifiers shall be distributed to a particular scan plan on
the distribute_qual_to_rels(); Its current criteria to distribute them is
"as deep as possible" according to the references of qualifiers.
This criteria cause a problem as I tried to tackle on the part.2 patch,
so it tries to put security-barrier to prevent unexpected pushing-down.

After the distribution, a scan plan will have qualifiers come from different
nest-levels. The order_qual_clauses() reorders the qualifiers according
to its cost estimation, so it possibly launches qualifiers come from upper
level prior to deeper one.

>> >> In addition, implementation will become complex, if both of qualifiers pulled-up
>> >> from security barrier view and qualifiers pulled-up from regular views are mixed
>> >> within a single qualifier list.
>> >
>> > I only scanned the part 2 patch, but isn't the bookkeeping already happening for
>> > its purposes? ?How much more complexity would we get to apply the same strategy
>> > to the behavior of this patch?
>> >
>> If conditional, what criteria we should have to reorder the quelifier?
>> The current patch checks the depth at first, then it checks cost if same deptn.
>> It is quite simple rule. I have no idea of the criteria to order the
>> mixed qualifier
>> come from security-barrier views and regular views.
>
> 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.

> 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?
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>