Re: INSERT ... ON CONFLICT UPDATE and RLS

* Dean Rasheed (dean(dot)a(dot)rasheed(at)gmail(dot)com) wrote:
> Turns out it wasn't as simple as that. prepend_row_security_policies()
> really could get called multiple times for the same RTE, because the
> call to query_tree_walker() at the end of fireRIRrules() would descend
> into the just-added quals again. The simplest fix seems to be to
> process RLS in a separate loop at the end, so that it can have it's
> own infinite recursion detection, which is different from that needed
> for pre-existing security quals and with check options from security
> barrier views. This approach simplifies things a bit, and ensures that
> we only try to expand RLS once for each RTE.

Right, I specifically recall having prepend_row_security_policies()
getting called multiple times for the same RTE. I like this approach of
using a separate loop though and it strikes me that it lends more
weight to the argument that we're better off with these as independent
considerations.

> > 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.

Hmm, good point.

> Turns out that refactoring actually became necessary in order to fix
> this bug, but I think it makes things cleaner and more efficient.

Sounds good, I'll take a look.

> Here's an updated patch with a new test for this bug. I've been
> developing the fixes for these RLS issues as one big patch, but I
> suppose it would be easy to split up, if that's preferred.

I'm alright with it as-is for now.

Thanks!

Stephen