Re: WIP patch (v2) for updatable security barrier views

Dean, Craig, all,

* Dean Rasheed (dean(dot)a(dot)rasheed(at)gmail(dot)com) wrote:
> On 11 April 2014 04:04, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > which changes it to "if a relation was changed to a subquery, it had
> > better be because it's got securityQuals that we're dealing with". My
> > general thinking here is that we'd be better off with the Assert()
> > firing during some later development which changes things in this area
> > than skipping the change because there aren't any securityQuals and then
> > expecting everything to be fine with the subquery actually being a
> > relation..
> >
>
> Yes, I think that makes sense, and it seems like a sensible check.

I've made this change and updated the comments a bit.

> For now though, the comments are correct, and it can only happen with
> securityQuals. Adding an Assert() to that effect might be a bit fiddly
> though, because the information required isn't available on the local
> Node being processed, so I think it would have to add and maintain an
> additional flag on the mutator context. I'm not sure that it's worth
> it.

Added a comment to that effect.

> > Also, it seems like there should be a check_stack_depth() call here now?
>
> Hm, perhaps. I don't see any such checks in the rewriter though, and I
> wouldn't expect the call depth here to get any deeper than it had
> previously done there.

I didn't do this. I tend to agree with you, though it'd be interesting
to see if it's possible to break things with enough levels... I suspect
that if we have a problem there though that it's in existing releases.

> Am I right in thinking that the "locking gotcha" only happens if you
> create a security_barrier view conaining a "SELECT ... FOR UPDATE"? If
> so, that seems like rather a niche case - not that that means we
> shouldn't warn people about it.

I've added both C-level comments and a new paragraph to the 'updatable
views' area of the documentation which attempt to address the issue
here- please review and comment.

I also went over prepsecurity.c and the regression tests and didn't
really find much to complain about there. I should be able to look at
the RLS patch tomorrow.

Thanks,

Stephen