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

On 3/9/22 16:06, Laurenz Albe wrote:
> This paragraph contains a couple of grammatical errors.
> How about
>
> <para>
> Note that the user performing the insert, update or delete on the view
> must have the corresponding insert, update or delete privilege on the
> view. Unless <literal>security_invoker</literal> is set to
> <literal>true</literal>, the view's owner must additionally have the
> relevant privileges on the underlying base relations, but the user
> performing the update does not need any permissions on the underlying
> base relations (see <xref linkend="rules-privileges"/>).
> If <literal>security_invoker</literal> is set to <literal>true</literal>,
> it is the invoking user rather than the view owner that must have the
> relevant privileges on the underlying base relations.
> </para>

Replaced the two paragraphs with your suggestion, it is indeed easier to
read.

>
> Also, this:
>
> [..]
>
> could be written like this (introducing a new variable):
>
> if (rule->event == CMD_SELECT
> && relation->rd_rel->relkind == RELKIND_VIEW
> && RelationHasSecurityInvoker(relation))
> user_for_check = InvalidOid;
> else
> user_for_check = relation->rd_rel->relowner;
>
> setRuleCheckAsUser((Node *) rule->actions, user_for_check);
> setRuleCheckAsUser(rule->qual, user_for_check);
>
> This might be easier to read.

Makes sense, I've changed that. This also seems to be more in line with
all the other code.
While at it I also split the comment alongside it to match, hopefully
that makes sense.

Thanks,
Christoph Heiss