Re: [v9.2] Fix leaky-view problem, part 1 - Mailing list pgsql-hackers
From | Noah Misch |
---|---|
Subject | Re: [v9.2] Fix leaky-view problem, part 1 |
Date | |
Msg-id | 20110628150054.GA10430@tornado.leadboat.com Whole thread Raw |
In response to | [v9.2] Fix leaky-view problem, part 1 (Kohei Kaigai <Kohei.Kaigai@EMEA.NEC.COM>) |
Responses |
Re: [v9.2] Fix leaky-view problem, part 1
|
List | pgsql-hackers |
I took a look at this patch. It's incredibly simple, which is great, and it seems to achieve its goal. Suppose your query references two views owned by different roles. The quals of those views will have the same depth. Is there a way for information to leak from one view owner to another due to that? I like how you've assumed that the table owner trusts the operator class functions of indexes on his table to not leak information. That handily catches some basic and important qual pushdowns that would otherwise be lost. This makes assumptions, at a distance, about the possible scan types and how quals can be fitted to them. Specifically, this patch achieves its goals because any indexable qual is trustworthy, and any non-indexable qual cannot be pushed down further than the view's own quals. That seems to be true with current plan types, but it feels fragile. I don't have a concrete idea for improvement, though. Robert suggested another approach in http://archives.postgresql.org/message-id/AANLkTimbN_6tYxReh5Rc7pmizT-VJB3xgp8CuHO0OAHC@mail.gmail.com ; might that approach avoid this hazard? The part 2 patch in this series implements the planner restriction more likely to yield performance regressions, so it introduces a mechanism for identifying when to apply the restriction. This patch, however, applies its restriction unconditionally. Since we will inevitably have a such mechanism before you are done sealing the leaks in our view implementation, the restriction in this patch should also use that mechanism. Therefore, I think we should review and apply part 2 first, then update this patch to use its conditional behavior. A few minor questions/comments on the implementation: On Mon, Jun 06, 2011 at 01:37:11PM +0100, Kohei Kaigai wrote: > --- a/src/backend/optimizer/plan/planner.c > +++ b/src/backend/optimizer/plan/planner.c > + else if (IsA(node, Query)) > + { > + depth += 2; Why two? > --- a/src/test/regress/expected/select_views.out > +++ b/src/test/regress/expected/select_views.out > +EXPLAIN SELECT * FROM your_credit WHERE f_leak(number,expired); > + QUERY PLAN > +-------------------------------------------------------------------------- > + Seq Scan on credit_cards (cost=0.00..181.20 rows=1 width=96) Use "EXPLAIN (COSTS OFF)" in regression tests. We do not put much effort into the stability of exact cost values, and they do not matter for the purpose of this test. > --- a/src/test/regress/sql/select_views.sql > +++ b/src/test/regress/sql/select_views.sql > @@ -8,3 +8,46 @@ SELECT * FROM street; > SELECT name, #thepath FROM iexit ORDER BY 1, 2; > > SELECT * FROM toyemp WHERE name = 'sharon'; > + > +-- > +-- Test for leaky-view problem > +-- > + > +-- setups > +SET client_min_messages TO 'warning'; > + > +DROP ROLE IF EXISTS alice; > +DROP FUNCTION IF EXISTS f_leak(text); > +DROP TABLE IF EXISTS credit_cards; > + > +RESET client_min_messages; No need for this. The regression tests always start on a clean database. > + > +CREATE USER alice; > +CREATE FUNCTION f_leak(text, text) > + RETURNS bool LANGUAGE 'plpgsql' > + AS 'begin raise notice ''% => %'', $1, $2; return true; end'; I ran this test case on master, and it did not reproduce the problem. However, adding "COST 0.1" to this CREATE FUNCTION did yield the expected problem behavior. I suggest adding that. Thanks, nm
pgsql-hackers by date: