Re: [v9.2] Fix Leaky View Problem

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.

Here are some simple benchmarking with pgbench -S (scale factor 10,
shared_buffers=400MB, MacBook Pro laptop) with and without this stack
of patches. These aren't clear-cut enough to make me absolutely sure
that this patch causes a noticeable performance regression, but I
think it does, and I'm not at all sure that this is the worst case:

results.kaigai.1:tps = 9359.908769 (including connections establishing)
results.kaigai.1:tps = 9366.317857 (including connections establishing)
results.kaigai.1:tps = 9413.593349 (including connections establishing)
results.master.1:tps = 9444.494510 (including connections establishing)
results.master.1:tps = 9400.486860 (including connections establishing)
results.master.1:tps = 9472.220529 (including connections establishing)

In the light of these problems, it doesn't seem worthwhile for me to
spend any more time on this right now: it looks to me like this needs
a lot more work before it can be considered for commit. I will mark
it Waiting on Author for now, but I think Returned with Feedback might
be more appropriate. This needs more than light cleanup; it needs
much more rigorous testing, both as to correctness and performance,
and at least a partial redesign.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company