On 17 March 2015 at 23:25, Peter Geoghegan <pg@heroku.com> wrote:
>> Possibly I'm missing something though.
>
> I think that you may have. Did you read the commit message/docs of the
> RLS commit 0004-*? You must consider the second point here, I believe:
>
> """"
> The 3 places that RLS policies are enforced are:
>
> * Against row actually inserted, after insertion proceeds successfully
> (INSERT-applicable policies only).
>
> * Against row in target table that caused conflict. The implementation
> is careful not to leak the contents of that row in diagnostic
> messages (INSERT-applicable *and* UPDATE-applicable policies).
>
> * Against the version of the row added by to the relation after
> ExecUpdate() is called (INSERT-applicable *and* UPDATE-applicable
> policies).
>
> """"
>
Yes, I read that, and I agree with the intention to not leak data
according to both the INSERT and UPDATE policies, however...
> You're seeing a failure that applies to the target tuple of the UPDATE
> (the tuple that we can't leak the contents of). I felt it was best to
> check all policies against the target/existing tuple, including both
> WITH CHECK OPTIONS and USING quals (which are both enforced).
>
I think that's an incorrect implementation of the RLS UPDATE policy.
The WITH CHECK quals of a RLS policy are intended to be applied to the
NEW data, not the existing data. This patch is applying the WITH CHECK
quals to both the existing and NEW tuples, which runs counter to the
way RLS polices are normally enforced, and I think that will just lead
to confusion.
> I can see why you might not like that behavior, but it is the intended
> behavior. I thought that this whole intersection of RLS + UPSERT is
> complex enough that it would be best to be almost as conservative as
> possible in what fails and what succeeds. The one exception is when
> the insert path is actually taken, since the statement is an INSERT
> statement.
The problem with that is that the user will see errors saying that the
data violates the RLS WITH CHECK policy, when they might quite
reasonably argue that it doesn't. That's not really being
conservative. I'd argue it's a bug.
Regards,
Dean
