Re: [v9.2] Fix Leaky View Problem

From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Noah Misch <noah(at)leadboat(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Thom Brown <thom(at)linux(dot)com>, Kohei Kaigai <Kohei(dot)Kaigai(at)emea(dot)nec(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [v9.2] Fix Leaky View Problem
Date: 2011-09-27 08:05:38
Message-ID: CADyhKSWoQrtRXKHpcfRHaT-do+RrxDgrAJV7S3C6RDQeMr2=zw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2011/9/26 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Mon, Sep 12, 2011 at 3:31 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>> The Part-2 tries to tackles a leaky-view scenarios by functions with
>> very tiny cost
>> estimation value. It was same one we had discussed in the commitfest-1st.
>> It prevents to launch functions earlier than ones come from inside of views with
>> "security_barrier" option.
>>
>> The Part-3 tries to tackles a leaky-view scenarios by functions that references
>> one side of join loop. It prevents to distribute qualifiers including
>> functions without
>> "leakproof" attribute into relations across security-barrier.
>
> I took a little more of a look at this today.  It has major problems.
>
> First, I get compiler warnings (which you might want to trap in the
> future by creating src/Makefile.custom with COPT=-Werror when
> compiling).
>
> Second, the regression tests fail on the select_views test.
>
> Third, it appears that the part2 patch works by adding an additional
> traversal of the entire query tree to standard_planner().  I don't
> think we want to add overhead to the common case where no security
> views are in use, or at least it had better be very small - so this
> doesn't seem acceptable to me.
>
The reason why I put a walker routine on the head of standard_planner()
was that previous revision of this patch tracked strict depth of sub-queries,
not a number of times to go through security barrier.
The policy to count-up depth of qualifier was changed according to Noad's
suggestion is commit-fest 1st, however, the suitable position to mark the
depth value was kept.
I'll try to revise the suitable position to track the depth value. It seems to
me one candidate is pull_up_subqueries during its recursive call, because
this patch originally set FromExpr->security_barrier here.

In addition to the two points you mentioned above, I'll update this patch
as follows:
* Use CREATE [SECURITY] VIEW statement, instead of reloptions.
the flag shall be stored within a new attribute of pg_class, and it shall
be kept when an existing view getting replaced.

* Utilize RangeTblEntry->relid, instead of rte->security_barrier, and the
flag shall be pulled from the catalog on planner stage.

* Documentation and Regression test.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kohei KaiGai 2011-09-27 08:13:17 Re: contrib/sepgsql regression tests are a no-go
Previous Message Simon Riggs 2011-09-27 07:00:50 Re: bug of recovery?