Re: [PATCH] Add reloption for views to enable RLS

Hi Laurenz,

thanks for the review!
I've attached a v2 where I addressed the things you mentioned.

On 1/11/22 19:59, Laurenz Albe wrote:
> [..]
> 
> You made that an enum with only a single value.
> What other values could you imagine in the future?
> 
> I think that this should be a boolean reloption, for example "security_definer".
> If unset or set to "off", you would get the current behavior.

A boolean option would have been indeed the better choice, I agree.
I haven't though of any specific other values for this enum, it was 
rather a decision following a off-list discussion.

I've changed the option to be boolean and renamed it to 
"security_invoker". This puts it in line with how other systems (e.g. 
MySQL) name their equivalent feature, so I think this should be an 
appropriate choice.

> 
>> Finally, patch 0003 updates the documentation for this new reloption.
> 
> [..]
> 
> Please avoid long lines like that.  

Fixed.

> Also, I don't think that the documentation on
> RLS policies is the correct place for this.  It should be on a page dedicated to views
> or permissions.
> 
> The CREATE VIEW page already has a paragraph about this, starting with
> "Access to tables referenced in the view is determined by permissions of the view owner."
> This looks like the best place to me (and it would need to be adapted anyway).
It makes sense to put it there, thanks for the pointer! I wasn't really 
that sure where to put the documentation to start with, and this seems 
like a more appropriate place.

Please review further.

Thanks,
Christoph Heiss