Re: WIP patch (v2) for updatable security barrier views
От | Dean Rasheed |
---|---|
Тема | Re: WIP patch (v2) for updatable security barrier views |
Дата | |
Msg-id | CAEZATCUiKxOg=vOOvjA2S6G-sixzzxg18ToTggP8zOBq6QnQHQ@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: WIP patch (v2) for updatable security barrier views (Dean Rasheed <dean.a.rasheed@gmail.com>) |
Ответы |
Re: WIP patch (v2) for updatable security barrier views
(Tom Lane <tgl@sss.pgh.pa.us>)
Re: WIP patch (v2) for updatable security barrier views (Craig Ringer <craig@2ndquadrant.com>) |
Список | pgsql-hackers |
On 8 January 2014 10:19, Dean Rasheed <dean.a.rasheed@gmail.com> wrote: > The assertion failure with inheritance and sublinks is a separate > issue --- adjust_appendrel_attrs() is not expecting to find any > unplanned sublinks in the query tree when it is invoked, since they > would normally have all been planned by that point. However, the > addition of the new security barrier subqueries after inheritance > expansion can now insert new sublinks which need to be planned. I'll > look into how best to make that happen. > Actually that wasn't quite it. The problem was that an RTE in the top-level query had a security barrier qual with a sublink in it, and it wasn't preprocessing those quals, so that sublink was still unplanned by the time it got to adjust_appendrel_attrs(), which then complained. 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. This doesn't affect performance in the normal case, because all other sublinks in the query will have been turned into subplans by that point, so it only needs to handle unplanned sublinks in RTE security barrier quals, which can only happen for updates to s.b. views. The attached patch does that, which fixes the case you reported. > On 8 January 2014 09:02, Craig Ringer <craig@2ndquadrant.com> wrote: >> My main concern is that the securityQuals appear to bypass all later >> rewrite stages, inheritance expansion during planning, etc. I suspect >> this might be hard to get around (because these are disembodied quals >> which may have nonsense varnos), but I'm looking into it now. >> Actually that's not the case. The securityQuals are traversed in the standard walker/mutator functions, so the rewriter *will* recursively expand any view references they contain (provided the query is correctly marked with hasSubLinks, which my first patch failed to do!). Inheritance expansion of relations in subqueries in the securityQuals is handled by recursion in the planner --- subquery_planner() invokes grouping_planner(), expanding securityQuals into new subquery RTEs, then subquery_planner() is recursively invoked for the new RTE subquery, which preprocesses its quals, which recursively invokes subquery_planner() for the sublinks in those quals, which then expands the inheritance sets they contain. >> My understanding from reading the patch is that this: >> >> - Flattens target views in rewriteTargetView, as in current master. If >> the target view is a security barrier view, the view quals are appended >> to a list of security barrier quals on the new RTE, instead of appended >> to the RTE's normal quals like for normal views. >> Right. >> After rewrite the views are fully flattened down to a RTE_RELATION, >> which becomes the resultRelation. An unreferenced RTE for each view >> that's been rewritten is preserved in the range-table for permissions >> checking purposes only (same as current master). >> Right. >> - Inheritance expansion, tlist expansion, etc then occurrs as normal. >> Right. >> - In planning, in inheritance_planner, if any RTE has any stashed >> security quals in its RangeTableEntry, expand_security_qual is invoked. >> This iteratively wraps the base relation in a subquery with the saved >> security barrier quals, creating nested subqueries around the original >> RTE. At each pass resultRelation is changed to point to the new >> outer-most subquery. >> Actually the resultRelation is only changed in the first pass. Each subsequent pass that creates an additional nested subquery RTE modifies the old subquery RTE in-place. The new subquery has an "identity" targetlist, which means that no further rewriting of the outer query is necessary after the first s.b. subquery is created. This avoids having multiple levels of attribute rewriting in the case where s.b. views are nested on top of one another. >> As a result of this approach everything looks normal to >> preprocess_targetlist, row-marking, etc, because they're seeing a normal >> RTE_RELATION as resultRelation. The security barrier quals are, at this >> stage, stashed aside. If there's inheritance involved, RTEs copied >> during appendrel expansion get copies of the security quals on in the >> parent RTE. >> >> Problem with inheritance, views, etc in s.b. quals >> -------------------------------------------------- >> >> After inheritance expansion, tlist expansion, etc, the s.b. quals are >> unpacked to create subqueries wrapping the original RTEs. >> >> >> So, with: >> >> CREATE TABLE t1 (x float, b integer, secret1 text, secret2 text); >> CREATE TABLE t1child (z integer) INHERITS (t1); >> >> INSERT INTO t1 (x, b, secret1, secret2) >> VALUES >> (0,0,'secret0', 'supersecret'), >> (1,1,'secret1', 'supersecret'), >> (2,2,'secret2', 'supersecret'), >> (3,3,'secret3', 'supersecret'), >> (4,4,'secret4', 'supersecret'), >> (5,6,'secret5', 'supersecret'); >> >> INSERT INTO t1child (x, b, secret1, secret2, z) >> VALUES >> (8,8,'secret8', 'ss', 8), >> (9,9,'secret8', 'ss', 9), >> (10,10,'secret8', 'ss', 10); >> >> CREATE VIEW v1 >> WITH (security_barrier) >> AS >> SELECT b AS b1, x AS x1, secret1 >> FROM t1 WHERE b % 2 = 0; >> >> CREATE VIEW v2 >> WITH (security_barrier) >> AS >> SELECT b1 AS b2, x1 AS x2 >> FROM v1 WHERE b1 % 4 = 0; >> >> >> >> then a statement like: >> >> UPDATE v2 >> SET x2 = x2 + 32; >> >> will be rewritten into something like (imaginary sql) >> >> UPDATE t1 WITH SECURITY QUALS ((b % 2 == 0), (b % 4 == 0)) >> SET x = x + 32 >> >> inheritance-expanded and tlist-expanded into something like (imaginary SQL) >> >> >> UPDATE >> (t1 WITH SECURITY QUALS ((b % 2 == 0), (b % 4 == 0))) >> UNION ALL >> (t1child WITH SECURITY QUALS ((b % 2 == 0), (b % 4 == 0))) >> SET x = x + 32; >> >> >> after which security qual expansion occurs, giving us something like: >> >> >> UPDATE >> t1, t1child <--- resultRelations >> ( >> SELECT v2.ctid, v2.* >> FROM ( >> SELECT v1.ctid, v1.* >> FROM ( >> SELECT t1.ctid, t1.* >> FROM t1 >> WHERE b % 2 == 0 >> ) v1 >> WHERE b % 4 == 0 >> ) v2 >> >> UNION ALL >> >> SELECT v2.ctid, v2.* >> FROM ( >> SELECT v1.ctid, v1.* >> FROM ( >> SELECT t1child.ctid, t1child.* >> FROM t1child >> WHERE b % 2 == 0 >> ) v1 >> WHERE b % 4 == 0 >> ) v2 >> >> ) >> SET x = x + 32; >> >> >> Giving a plan looking like: >> >> EXPLAIN UPDATE v2 SET x2 = 32 >> >> >> QUERY PLAN >> --------------------------------------------------------------------------- >> Update on t1 t1_2 (cost=0.00..23.35 rows=2 width=76) >> -> Subquery Scan on t1 (cost=0.00..2.18 rows=1 width=74) >> -> Subquery Scan on t1_3 (cost=0.00..2.17 rows=1 width=74) >> Filter: ((t1_3.b % 4) = 0) >> -> Seq Scan on t1 t1_4 (cost=0.00..2.16 rows=1 width=74) >> Filter: ((b % 2) = 0) >> -> Subquery Scan on t1_1 (cost=0.00..21.17 rows=1 width=78) >> -> Subquery Scan on t1_5 (cost=0.00..21.16 rows=1 width=78) >> Filter: ((t1_5.b % 4) = 0) >> -> Seq Scan on t1child (cost=0.00..21.10 rows=4 width=78) >> Filter: ((b % 2) = 0) >> (11 rows) >> >> >> >> >> So far this looks like a really clever approach. My only real concern is >> that the security quals are currently hidden from rewrite and parsing >> before during the period they're hidden away in the security quals RTI >> field. >> >> This means they aren't processed for things like inheritance expansion. e.g. >> >> CREATE TABLE rowfilter (remainder integer, userid text); >> CREATE TABLE rowfilterchild () INHERITS (rowfilter); >> INSERT INTO rowfilterchild(remainder, userid) values (0, current_user); >> >> a view with a security qual that refers to an inherited relation won't work: >> >> CREATE VIEW sqv >> WITH (security_barrier) >> AS >> SELECT x, b FROM t1 WHERE ( >> SELECT b % 4 = remainder >> FROM rowfilter >> WHERE userid = current_user >> OFFSET 0 >> ); >> >> This is a bit contrived to force the optimiser to treat the subquery as >> correlated and thus make sure the ref to rowfilter gets into the >> securityQuals list. >> >> I expected zero results (a scan of rowfilter, but not rowfilterchild, >> resulting in a null subquery return) but land up with an assertion >> failure instead. The assertion triggers for any security qual containing >> a correlated subquery, so it's crashing out before we can break due to >> failure to expand inheritance. >> Having fixed the assertion failure by making adjust_appendrel_attrs() handle descent into sublink subqueries, this now works, and it does correctly expand the inheritance in the subquery in the s.b. qual, for the reasons explained above: explain (verbose, costs off) update sqv set b=b*10 where b%5=0; QUERY PLAN ------------------------------------------------------------------------------------------------------- Update on public.t1 t1_2 -> Subquery Scan on t1 Output: t1.x, (t1.b * 10), t1.secret1, t1.secret2, t1.ctid Filter: ((t1.b % 5) = 0) -> Seq Scan on public.t1 t1_3 Output: t1_3.b, t1_3.ctid, t1_3.x, t1_3.secret1, t1_3.secret2 Filter: (SubPlan 1) SubPlan 1 -> Result Output: ((t1_3.b % 4) = rowfilter.remainder) -> Append -> Seq Scan on public.rowfilter Output: rowfilter.remainder Filter: (rowfilter.userid = ("current_user"())::text) -> Seq Scan on public.rowfilterchild Output: rowfilterchild.remainder Filter: (rowfilterchild.userid = ("current_user"())::text) -> Subquery Scan on t1_1 Output: t1_1.x, (t1_1.b * 10), t1_1.secret1, t1_1.secret2, t1_1.z, t1_1.ctid Filter: ((t1_1.b % 5) = 0) -> Seq Scan on public.t1child Output: t1child.b, t1child.ctid, t1child.x, t1child.secret1, t1child.secret2, t1child.z Filter: (SubPlan 2) SubPlan 2 -> Result Output: ((t1child.b % 4) = rowfilter_1.remainder) -> Append -> Seq Scan on public.rowfilter rowfilter_1 Output: rowfilter_1.remainder Filter: (rowfilter_1.userid = ("current_user"())::text) -> Seq Scan on public.rowfilterchild rowfilterchild_1 Output: rowfilterchild_1.remainder Filter: (rowfilterchild_1.userid = ("current_user"())::text) >> This isn't just about inheritance. In general, we'd need a way to >> process those securityQuals through any rewrite phases and through the >> parts of planning before they get merged back in, so they're subject to >> things like inheritance appendrel expansion. >> I believe that should work because the rewriter traverses the query tree applying rewrite rules to everything it sees, and that will include any securityQuals. In fact one of my concerns about your approach of expanding the s.b. view in rewriteTargetView() was how that would interact with later stages of the rewriter if there were more rules on the base relation. rewriteTargetView() happens very early in the rewriter, so there is potentially a lot more that can happen to the query tree after that, which would make it difficult to keep track of the relationship between the target RTE and expanded subquery RTE. Storing the securityQuals on the RTE keeps them tied together, which makes it easier to predict what will happen, although this still does need a lot more testing. Regards, Dean doc/src/sgml/ref/create_view.sgml | 6 src/backend/commands/tablecmds.c | 6 src/backend/commands/view.c | 6 src/backend/nodes/copyfuncs.c | 1 src/backend/nodes/equalfuncs.c | 1 src/backend/nodes/nodeFuncs.c | 4 src/backend/nodes/outfuncs.c | 1 src/backend/nodes/readfuncs.c | 1 src/backend/optimizer/plan/planner.c | 37 !! src/backend/optimizer/prep/Makefile | 2 src/backend/optimizer/prep/prepsecurity.c | 423 ++++++++++++++++++++++++++ src/backend/optimizer/prep/prepunion.c | 57 !!! src/backend/rewrite/rewriteHandler.c | 53 - src/include/nodes/parsenodes.h | 1 src/include/optimizer/prep.h | 5 src/include/rewrite/rewriteHandler.h | 1 src/test/regress/expected/create_view.out | 2 src/test/regress/expected/updatable_views.out | 261 +++++++++++!!! src/test/regress/sql/updatable_views.sql | 92 ++++! 19 files changed, 738 insertions(+), 25 deletions(-), 197 modifications(!)
Вложения
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Michael PaquierДата:
Сообщение: Re: In-core regression tests for replication, cascading, archiving, PITR, etc. Michael Paquier