Re: WIP patch (v2) for updatable security barrier views

On 01/21/2014 09:09 AM, KaiGai Kohei wrote:
> (2014/01/13 22:53), Craig Ringer wrote:
>> On 01/09/2014 11:19 PM, Tom Lane wrote:
>>> Dean Rasheed <dean.a.rasheed@gmail.com> writes:
>>>> My first thought was that it should just preprocess any security
>>>> barrier quals in subquery_planner() in the same way as other quals are
>>>> preprocessed. But thinking about it further, those quals are destined
>>>> to become the quals of subqueries in the range table, so we don't
>>>> actually want to preprocess them at that stage --- that will happen
>>>> later when the new subquery is planned by recursion back into
>>>> subquery_planner(). So I think the right answer is to make
>>>> adjust_appendrel_attrs() handle recursion into sublink subqueries.
>>>
>>> TBH, this sounds like doubling down on a wrong design choice.  I see
>>> no good reason that updatable security views should require any
>>> fundamental rearrangements of the order of operations in the planner.
>>
>> In that case, would you mind offerign a quick sanity check on the
>> following alternative idea:
>>
>> - Add "sourceRelation" to Query. This refers to the RTE that supplies
>> tuple projections to feed into ExecModifyTable, with appropriate resjunk
>> "ctid" and (if requ'd) "oid" cols present.
>>
>> - When expanding a target view into a subquery, set "sourceRelation" on
>> the outer view to the index of the RTE of the newly expanded subquery.
>>
> I have sane opinion. Existing assumption, "resultRelation" is RTE of the
> table to be read not only modified, makes the implementation complicated.
> If we would have a separate "sourceRelation", it allows to have flexible
> form including sub-query with security_barrier on the reader side.
> 
> Could you tell me the direction you're inclined right now?

My focus right now is getting your original RLS patch rebased on top of
head, separated into a series of patches that can be understood as
separate functional units, then rewritten on top of updatable security
barrier views.

( See
http://www.postgresql.org/message-id/52DCBEF1.3010004@2ndquadrant.com )

I don't really care which updatable security barrier views
implementation it is. Dean's has the advantage of actually working, so
I'm basing it on his.

It sounds like Tom objects to Dean's approach to some degree, but we'll
have to see whether that turns into concrete issues. If it does, it
should be possible to replace the underlying updatable security barrier
views implementation without upsetting the rewritten RLS significantly.
That's part of why I've gone down this path - it separates "filtering
rows according to security predicates" from the rest of RLS, into a
largely functionally separate patch.

> I wonder whether I should take the latest patch submitted on 09-Jan for
> a deep code reviewing and testing.

That would be extremely valuable. Please break it, or show that you
could not figure out how to break it.

If you find an unfixable flaw, we'll go back to the approach outlined in
this mail - split resultRelation, insert ctids down the subquery chain, etc.

If you don't find a major flaw, then that's one less thing to worry
about, and we can focus on the RLS layer.

-- Craig Ringer                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services