Re: INSERT ... ON CONFLICT UPDATE and RLS

On 10 January 2015 at 15:12, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>> 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.
>
> I agree that's an independent change. I'm not sure if it would end up
> being neater or not offhand but I can see some advantages to breaking it
> up from WithCheck as it might be simpler for users to understand
> (particularly those users who are not already familiar with WithCheck).

A new RlsCheck struct wouldn't need the cascade option that
WithCheckOption has, and it's not nice the way viewname is being
abused. For UPSERT it will need a field indicating the command type,
as Peter pointed out, so I think it's different enough to warrant it's
own struct, even if we weren't changing the firing time.

> Were you thinking about working up a patch for such a change?

OK, I'll take a look at it. I started reading the existing code that
expands RLS policies and spotted a couple of bugs that should be fixed
first:-

1). In prepend_row_security_policies(), defaultDeny should be
initialised to false at the top.

2). In fireRIRrules(), activeRIRs isn't being reset properly after
recursing, which will cause a self-join to fail.

Regards,
Dean