On 13 September 2015 at 20:59, Stephen Frost <sfr...@snowman.net> wrote:
> I've been through this and definitely like it more than what we had
> previously, as I had mentioned previously.  I've also added the
> RETURNING handling, per the discussion between you, Robert, Tom, and
> Kevin.
>
> Would be great to get your feedback both on the relatively minor changes
> which I made to your refactoring patch (mostly cosmetic) and how I added
> in the handling for the RETURNING case, which was made much simpler and
> cleaner by the refactoring.  (Working through adding the RETURNING also
> helped my understanding of the refactoring, which I feel comfortable
> that I understand well now.)
>

Cool. I haven't looked in detail yet, but I expected the changes
necessary to support RETURNING to be much simpler than that. Isn't it
just a case of adding something like

if (root->returningList &&
    commandType == CMD_UPDATE or CMD_DELETE)
{
    get_policies_for_relation(rel, CMD_SELECT, ...)
    build_security_quals(...)
}

and then something similar with build_with_check_options() for the upsert case?

Then it isn't necessary to modify get_policies_for_relation(),
build_security_quals() and build_with_check_options() to know anything
specific about returning. They're just another set of permissive and
restrictive policies to be fetched and added to the command.

One change I thought about making was renaming build_security_quals()
and build_with_check_options() to add_security_quals() and
add_with_check_options(), because they're adding to lists rather than
returning lists, and in general they are called multiple times to
build up the complete lists.

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

Reply via email to