Re: RLS open items are vague and unactionable

On 14 September 2015 at 14:47, Stephen Frost <sfrost@snowman.net> wrote:
> Attached is a git format-patch built series which includes both commits,
> now broken out, for review.
>

That looks OK to me.

A minor point -- this comment isn't quite right:
   /*    * For the target relation, when there is a returning list, we need to    * collect up CMD_SELECT policies to
addvia add_security_quals and    * add_with_check_options.  This is because, for the RETURNING case, we    * have to
filterany records which are not visible through an ALL or SELECT    * USING policy.    *    * We don't need to worry
aboutthe non-target relation case because we are    * checking the ALL and SELECT policies for those relations anyway
(see   * above).    */
 

because the policies that are fetched there are only used for
add_security_quals(), not for add_with_check_options(). It might be
cleaner if the 'if' statement that follows were merged with the
identical one a few lines down, and then those returning policies
could be local to that block, with the 2 pieces of RETURNING handling
done together. Similarly for the upsert block.

Actually, it isn't necessary to test that rt_index ==
root->resultRelation, because for all other relations commandType is
set to CMD_SELECT higher up, so the 'returning' bool variable could
just be replaced with 'root->returningList != NIL' throughout.

Regards,
Dean