Re: Row-security on updatable s.b. views - Mailing list pgsql-hackers

From Craig Ringer
Subject Re: Row-security on updatable s.b. views
Date
Msg-id 52F09946.9020907@2ndquadrant.com
Whole thread Raw
In response to Re: Row-security on updatable s.b. views  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On 02/04/2014 03:14 PM, Tom Lane wrote:
> Craig Ringer <craig@2ndquadrant.com> writes:
>> I landed up adding a field to RangeTblEntry that keeps track of all the
>> oids of relations row-security expanded to produce this RTE. When
>> testing an RTE for row-security policy, this list is checked to see if
>> the oid of the relation being expanded is already on the list. The list
>> is copied to all RTEs inside sublinks after a relation is expanded using
>> a query_tree_walker.
> 
> That's impossibly ugly, both as to the idea that an RTE is a mutable
> data structure for this processing, and as to having to copy the list
> around (meaning that you can't even claim to have one source of truth
> for the state...)

I see. Do you have any suggestion about how this might be approached in
a manner that would be more satisfactory?

It's not strictly necessary to duplicate the parent list when annotating
the RTEs in an expanded row-security qual; I did so only out of concern
that an object of indeterminate/shared ownership may not be considered
desirable.

The only time it's strictly neccessary to copy the parent list is when a
new row-security qual is being expanded. In this case it cannot be
shared, because there may be multiple relations with row-security
applied, and a separate "path" is required for each. If the list is
shared, infinite recursion is falsely reported in cases like:

rel a has a qual referring to b
rel b has a qual referring to c
rel c has a qual referring to d

select ...
from a inner join b on ...

There's no infinite recursion there, but a simple approach that keeps a
global list of expanded quals will think there is as it'll see "b" and
"c" expanded twice.

So whenever you expand a qual, you have to "fork" the parent list,
copying it.

The approach used in the rewriter won't work here, because the rewriter
eagerly depth-first recurses when expanding things. In the optimizer,
all securityQuals get expanded in one pass, _then_ sublink processing
expands any added sublinks in a separate pass.

It's possible the parent list could be associated with the Query that's
produced by securityQual expansion instead, but at the time you're
expanding row-security on any RTEs within that you don't necessarily
have access to the Query node that might be a couple of subquery levels
up, since the security qual can be an arbitrary expression.

I looked at keeping a structure in PlannerGlobal that tracked the chain
of row-security qual expansion, but I couldn't figure out a way to
cleanly tell what "branch" of the query a given RTE that's being
expanded was in, and thus how to walk the global tree to check for
infinite recursion.



The only alternative I see is to immediately and recursively expand
row-security entries within subqueries using a walker, as soon as the
initial row-security qual is expanded on the top level rel.

I'm concerned about code duplication when following that path, since
that's pretty much what's already happening with sublink expansion, just
using the existing execution paths.

If I'm right, and immediate recursive expansion isn't an acceptable
alternative, any ideas on how the row-security expansion breadcrumb
trail might be stored in a more appropriate manner and accessed from
where they're needed?

Note that it's trivial to prevent simple, direct recursion. Preventing
mutual recursion without also breaking non-recursive cases where rels
are used in totally different parts of the query is harder.

-- Craig Ringer                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Row-security on updatable s.b. views
Next
From: Amit Kapila
Date:
Subject: Re: Retain dynamic shared memory segments for postmaster lifetime