Re: INSERT ... ON CONFLICT UPDATE and RLS - Mailing list pgsql-hackers

On 9 January 2015 at 20:26, Stephen Frost <sfrost@snowman.net> wrote:
> Where this leaves me, at least, is feeling like we should always apply
> the INSERT WITH CHECK policy, then if there is a conflict, check the
> UPDATE USING policy and throw an error if the row isn't visible but
> otherwise perform the UPDATE and then check the UPDATE WITH CHECK
> policy.

Yeah, I can see the logic in that, but I'm starting to question the
final "then" in the above, i.e., the order in which the checks are
done.

Currently we're applying RLS CHECKs after the INSERT or UPDATE, like
WITH CHECK OPTIONs on views. The SQL spec says that WITH CHECK OPTIONs
on views have to be applied after the INSERT/UPDATE on the base
relation, but we're free to do something different for RLS CHECKs if
that makes more sense. If we want RLS to be more like column-level
privilege checking, then it does make sense to do the checks sooner,
so perhaps we should be checking the RLS policies before the
INSERT/UPDATE, like CHECK constraints.

Doing that, it makes sense to first check INSERT CHECK policies,
potentially throwing an error before any PK conflict is detected, so
an error would result even in the INSERT .. ON CONFLICT IGNORE case.
That way INSERT CHECK policies would always be applied regardless of
the path taken, as you suggest.

I would still say that the RLS UPDATE USING/CHECK policies should only
be applied if the UPDATE path is taken (before the UPDATE is applied),
and they need to be applied to the (old/new) update tuples only. It's
a bit like there is a WHERE <record already exists> clause attached to
the UPDATE. The RLS UPDATE policies should only be applied to the rows
affected by the UPDATE.

> I see your point that this runs counter to the 'mod_count'
> example use-case and could cause problems for users using RLS with such
> a strategy.  For my part, I expect users of RLS to expect errors in such
> a case rather than allowing it, but it's certainly a judgement call.
>

Actually I don't think this is a problem for the mod_count example.
This isn't the same as AND'ing together the INSERT and UPDATE quals,
because each set of policies is being applied to a different tuple.
There are 3 tuples in play here:-

1). The insert tuple, which has INSERT CHECK polcies applied to it
2). The existing (old) update tuple, which has UPDATE USING policies
applied to it
3). The (new) update tuple, which has UPDATE CHECK policies applied to it


NOTE: If we do change RLS CHECKs to be executed prior to modifying any
data, that's potentially a change that could be made independently of
the UPSERT patch. We should also probably then stop referring to them
as WITH CHECK OPTIONs in the docs and in error messages because
they're not the same thing, and the code might be neater if they had
their own data-structure rather than overloading WithCheckOption.

Regards,
Dean