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