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