Updatable security_barrier views (was: [v9.4] row level security)

On 11/12/2013 05:40 AM, Robert Haas wrote:
> I haven't studied this issue well enough to know what's really needed
> here, but Dean Rasheed's approach sounded like a promising tack to me.

I've been looking further into adding update support for security
barrier views and after reading the code for UPDATE ... FROM I don't
understand why there was any need to add separate targetRelation and
sourceRelation plan attributes.

UPDATE ... FROM already operates on the target relation specified in the
:resultRelation of the QUERY node; it's a index into the range-table.

It's already quite happy getting its input rows from a tree of joins and
other plan nodes; all it seems to need is the ctid, any old values of
columns to be emitted via RETURNING, and the expressions for any new
column values in the top-level query.

It looks like

UPDATE t SET t.somecol = t2.othercol
FROM t2
WHERE t.id = t2.id;

is already handled as if it were the (imaginary, but hopefully self
explanatory) sql:

UPDATE (
SELECT t.ctid AS t_ctid, t2.othercol AS othercol
FROM t
INNER JOIN t2 ON t.id = t2.id
) sq
TARGET t
SET t.othercol = sq.othercol
WHERE t.ctid = sq.ctid;

where the :resultRelation identifies the RTI of the rel that the new
tuples should be added to and that old tuples should have their xmax set in.

If my understanding is vaguely right, adding sourceRelation and
targetRelation should not be necessary. We just have to transform the
query tree at the appropriate stage, so that a query over a security
barrier view emits a plan like this:

- resultRelation points to the RTE of the base relation discovered
by recursively scanning S.B. subqueries and added to the RangeTable.
This is the underlying RTE_RELATION. It should already be in the
RangeTable but might need to be copied to appear with different
required permissions for use in the ModifyTable node.

- S.B. subqueries in the plan are rewritten to add the ctid column
as resjunk. This is much the same as what happens when a MergeJoin,
HashJoin, or NestLoop node is under the ModifyTable - they're
adjusted to add the ctid column by expand_targetlist in
backend/optimizer/prep/preptlist.c . This is analogous to the
current attr/var fixups in RLS and Dean's patch, but it should
hopefully be possible to do it more cleanly in expand_targetlist.
It's the biggest question mark so far.

- The test for subquery as result relation in preprocess_targetlist
(backend/optimizer/prep/preptlist.c)
remains in place. The subquery isn't the result relation, the
underlying RTE_RELATION the stack of subqueries are based on is.

- When the updatable view code sees a s.b. view, expand it like a normal
view but don't pull up the quals and flatten out the subquery. May
need to update :resultRelation.

The executor will pull rows from the plan - running nested subquery
nodes, etc - and will get result tuples containing the ctid of the tuple
in the base rel, the old values of the fields in the row if needed for
RETURNING, and any computed cols for new values.

That will feed into the ModifyTable node, which will use the
:resultRelation just like it did before, without caring in the slightest
that the inputs came from a subquery.

I'm going to try to implement it as an experiment, see if I can make it
work. If I'm totally barking up the wrong tree or have missed something
major, please feel free to say so.

"simply updatable" views already include those that contain a subquery
over another table, just not directly in FROM, eg:

regress=> CREATE VIEW t_even_sq AS SELECT t.* FROM t
WHERE EXISTS (SELECT 1 FROM t2 WHERE t.id = t2.id);
CREATE VIEW
regress=> UPDATE t_even_sq SET id = id;
UPDATE 10

so this won't stop row-security from using subqueries to refer to other
relations in access tests.

Determining which rel to update in the presence of subqueries over other
tables should just involve making sure the right :resultRelation is kept
track of during recursive view expansion.

Totally crazy? Or workable? I'm extremely new to the planner, so I know
this might be unworkable, and would value advice.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services