On Mon, Sep 12, 2011 at 3:31 PM, Kohei KaiGai <kai...@kaigai.gr.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers