Re: INSERT ... ON CONFLICT UPDATE and RLS

On 10 January 2015 at 21:38, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
> OK, I'll take a look at it. I started reading the existing code that
> expands RLS policies and spotted a couple of bugs that should be fixed
> first:-
>
> 1). In prepend_row_security_policies(), defaultDeny should be
> initialised to false at the top.
>
> 2). In fireRIRrules(), activeRIRs isn't being reset properly after
> recursing, which will cause a self-join to fail.
>

So as I starting looking into these bugs, which looked fairly trivial,
the test case I came up with revealed another more subtle bug with
RLS, using a more obscure query -- given an update of the form UPDATE
t1 ... FROM t2 ..., if t1 is part of an inheritance hierarchy and both
t1 and t2 have RLS enabled, the inheritance planner was doing the
wrong thing. There is code in there to copy subquery RTEs into each
child plan, which needed to do the same for non-target RTEs with
security barrier quals, which haven't yet been turned into subqueries.
Similarly the rowmark preprocessing code needed to be taught about
(non-target) RTEs with security barrier quals to handle this kind of
UPDATE with a FROM clause.

The attached patch fixes those issues.

This bug can't happen with updatable security barrier views, since
non-target security barrier views are expanded in the rewriter, so
technically this doesn't need bach-patching to 9.4. However, I think
the planner changes should probably be back-patched anyway, to keep
the code in the 2 branches in sync, and more maintainable. Also it
feels like the planner ought to be able to handle any legal query
thrown at it, even if it looks like the 9.4 rewriter couldn't generate
such a query.

Regards,
Dean