Re: INSERT ... ON CONFLICT UPDATE and RLS

* Dean Rasheed (dean(dot)a(dot)rasheed(at)gmail(dot)com) wrote:
> On 6 January 2015 at 20:44, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> > Another issue that I see is that there is only one resultRelation
> > between the INSERT and UPDATE. That means that without some additional
> > special measure, both UPDATE and INSERT "WITH CHECK ( ... ) " options
> > are applied regardless of whether the INSERT path was taken, or the
> > UPDATE path. Maybe that's actually sensible (note that even this
> > doesn't happen right now, since the auxiliary UPDATE is currently
> > minimally processed by the rewriter). What do people think about that?
> > It seems like it might be less surprising to have that fail earlier
> > when there are separate policies for INSERT and UPDATE -- for UPSERT,
> > all policies are applied regardless of which path is taken.
>
> I think the policies applied should depend on the path taken, so if it
> does an INSERT, then only the INSERT CHECK policy should be applied
> (after the insert), but if it ends up doing an UPDATE, I would expect
> the UPDATE USING policy to be applied (before the update) and the
> UPDATE CHECK policy to be applied (after the update). I would not
> expect the INSERT CHECK policy to be applied on the UPDATE path.

You're making a distinction where I'm not convinced there should be one.
While I understand that, internally, there are two paths (INSERT vs.
UPDATE), it's still only one command for the user and their goal is
simply "update if this exists, insert if it doesn't." I also don't like
the idea that the policy to be applied depends on if it ends up being an
INSERT or an UPDATE. I liked Peter's suggestion that the row must pass
both WITH CHECK conditions- at least that's consistent and
understandable.

> As to whether the UPDATE USING policy should cause an error to be
> thrown if it is not satisfied, my inclination would be to not error,
> and use the command tag to report that no rows were updated, since
> that is what would happen with a regular UPDATE.

Yes, for a regular UPDATE that's what would happen- but this isn't a
regular UPDATE, it's an UPSERT. Consider how users handle this now-
they have routines which continually try to do one or the other until
either the INSERT or the UPDATE is successful. I've never seen such a
function, properly written, that says "try to INSERT, if that fails, try
to UPDATE and if that doesn't update anything, then simply exit." What
is the use-case for that?

> So overall INSERT .. ON CONFLICT UPDATE would be consistent with
> either an INSERT or an UPDATE, depending on whether the row existed
> beforehand, which is easier to explain than having some special UPSERT
> semantics.

Any semantics which result in a no-op would be surprising for users of
UPSERT. I like the idea of failing earlier- if you try to INSERT .. ON
CONFLICT UPDATE a row which you're not allowed to INSERT, erroring
strikes me as the right result. If you try to INSERT .. ON CONFLICT
UPDATE a row which you're not allowed to UPDATE, I'd also think an error
would be the right answer, even if you don't end up doing an actual
UPDATE- you ran a command asking for an UPDATE to happen under a certain
condition (the row already exists) and the policy states that you're not
allowed to do such an UPDATE.

As for the UPDATE's USING clause, if you're not allowed to view the
records to be updated (which evidently exists because the INSERT
failed), I'd handle that the same way we handle the case that a SELECT
policy might not allow a user to see rows that they can attempt to
INSERT- they'll get an error back in that case too.

In the end, these are edge cases as policies are generally self
consistent and so I think we have some leeway, but I don't think we have
the right to turn an INSERT .. ON CONFLICT UPDATE into a no-op. That
would be like allowing an INSERT to be a no-op.

Thanks,

Stephen