Re: Prohibit row-security + inheritance in 9.4?

On 01/31/2014 11:24 PM, Yeb Havinga wrote:
> On 2014-01-31 16:05, Stephen Frost wrote:
>> * Yeb Havinga (y(dot)t(dot)havinga(at)mgrid(dot)net) wrote:
>>> IMHO, there is another way to implement this, other than the
>>> procedure to override the child-rel-quals with the ones from the
>>> parent. At DDL time, synchronize quals on the parent with rls quals
>>> of the childs. Isn't this also what happens with constraints?
>> No, we're not going to do that. We don't do it for GRANT and I don't
>> think it makes sense to do it here.
> This reasoning could go either way. GRANT is on a complete set of rows.
> This is a restriction on the level of individual rows, and in that
> sense, it is more like a row-level CHECK constraint.
>> If we wanted to make them the same then we'd throw out the ability to do
>> any kind of changes or actions on the child and then we'd have actual
>> partitioning. We don't have that though, we have inheiritance.
>
> I fail to understand this, probably because I do not have a partition
> use case for inheritance, but rather an information model that is more
> ontology like. The more specific childs get down the inheritance tree,
> more columns they get, and their requirements might differ completely in
> nature from their siblings, and make no sense at all as well when
> specified at the level of the parent (or even impossible, since the
> parent does not have all the columns).

That's a bit inconsistent with how Pg's inheritance works, though.
Conceptually rows in children *are* part of the parent.

You cannot see the child columns when querying via the parent, so it's
not a problem to constrain the ability to see the extra child columns
with row-security. They can only be accessed when querying the child
directly, where the child's row-security expression will apply.

So you're not exposing information that's specific to the children, and
the inherited common cols are, conceptually *part* of the parent, so
applying the parent's qual makes sense.

>>> Then during expansion of the range table, no code is needed to
>>> ignore child rls quals and copy parent rels to child rels.
>> This is what's already implemented and isn't a huge amount of code to
>> begin with, so I don't see this as being an argument against having the
>> flexibility.
>
> It would seem to me that any additional logic that can be avoided during
> planning is a good thing. Also, the argument that something is already
> implemented, does not itself make it good to commit.

The implementation with the minimum of required logic and complexity is
"apply the parent's row-security quals to the children, don't check
children for quals".

Any special handling of child rels creates exciting challenges because
inheritance expansion happens _after_ a bunch of the query planner and
optimizer has run. Injecting new expression trees at this point is
messy, especially if you want those expression trees to in turn contain
row-security qualified tables, inheritance, etc.

As previously discussed, applying the parent qual to children ensures
that what's visible when querying a relation that has children is
consistent with its row-security qual.

If the parent qual is applied only on the parent rel directly, not
children, then querying the parent could emit rows not permitted by the
parent's row-security qual. I'm not happy with that, and as Stephen
poined out upthread, it isn't really consistent with how permissions
work with inheritance otherwise.

If instead the parent qual is applied to the parent and all children,
and you *also* add the quals of each child, you get a complex, hard to
debug, hard to optimize mess. You also run back into the problem
mentioned above, that adding quals after inhertiance expansion is messy
and problematic. It's also really ugly in what's going to be the most
common case, where the child quals are the same as the parent quals, as
you'll get nested identical quals.

Despite the challenges with it, I think that's the least insane way to
respect child quals on parents. It has pretty much zero chance of
happening in 9.4, though; the discussed approach of building
row-security on top of updatable security barrier views doesn't play
well with adding inheritance on inheritance-expanded children.

One answer for that would be to keep inheritance as it is for 9.4 (if I
can get the remaining issues sorted out) and in 9.5, if possible, allow
the addition of a row-security qual that, if it appears on a child rel
during inheritance expansion, _is_ expanded. At least, if it proves
necessary, which I'm not entirely convinced of.

> I suggested it as a solution for a requirement worded upthread as "It
> makes absolutely zero sense, in my head anyway, to have rows returned
> when querying the parent which should NOT be returned based on the quals
> of the parent." without disregarding rls-quals on childs.

I'm not sure I understand what you are saying here.

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