On 12 January 2015 at 14:24, Stephen Frost <sfrost@snowman.net> wrote:
> Interesting, thanks for the work! I had been suspicious that there was
> an issue with the recursion handling.
>
So continuing to review the RLS code, I spotted the following in
prepend_row_security_policies():
/* * We may end up getting called multiple times for the same RTE, so check * to make sure we aren't doing
double-work. */ if (rte->securityQuals != NIL) return false;
which looked suspicious. I assume that's just a hang-up from an
earlier attempt to prevent infinite recursion in RLS expansion, but
actually it's wrong because in the case of an UPDATE to a security
barrier view on top of a table with RLS enabled, the view's
securityQuals will get added to the RTE first, and that shouldn't
prevent the underlying table's RLS securityQuals from being added.
AFAICT, it should be safe to just remove the above code. I can't see
how prepend_row_security_policies() could end up getting called more
than once for the same RTE.
Also, I'm thinking that it would be better to refactor things a bit
and have prepend_row_security_policies() just return the new
securityQuals and withCheckOptions to add. Then fireRIRrules() would
only have to recurse into the new quals being added, not the
already-processed quals.
Regards,
Dean