On 14 September 2015 at 14:47, Stephen Frost <sfr...@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 add via add_security_quals and * add_with_check_options. This is because, for the RETURNING case, we * have to filter any records which are not visible through an ALL or SELECT * USING policy. * * We don't need to worry about the 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers