On Mon, Apr 27, 2015 at 5:07 AM, Shigeru Hanada <shigeru.han...@gmail.com> wrote: > 2015-04-27 11:00 GMT+09:00 Kouhei Kaigai <kai...@ak.jp.nec.com>: >> Hanada-san, could you adjust your postgres_fdw patch according to >> the above new (previous?) definition. > > The attached v14 patch is the revised version for your v13 patch. It also > contains changed for Ashutosh’s comments.
We should probably move this discussion to a new thread now that the other patch is committed. Changing subject line accordingly. Generally, there's an awful lot of changes in this patch - it is over 2000 insertions and more than 450 deletions - and it's not awfully obvious why all of those changes are there. I think this patch needs a detailed README to accompany it explaining what the various changes in the patch are and why those things got changed; or maybe there is a way to break it up into multiple patches so that we can take a more incremental approach. I am really suspicious of the amount of wholesale reorganization of code that this patch is doing. It's really hard to validate that a reorganization like that is necessary, or that it's correct, and it's gonna make back-patching noticeably harder in the future. If we really need this much code churn it needs careful justification; if we don't, we shouldn't do it. +SET enable_mergejoin = off; -- planner choose MergeJoin even it has higher costs, so disable it for testing. This seems awfully strange. Why would the planner choose a plan if it had a higher cost? - * If the table or the server is configured to use remote estimates, - * identify which user to do remote access as during planning. This + * Identify which user to do remote access as during planning. This * should match what ExecCheckRTEPerms() does. If we fail due to lack of * permissions, the query would have failed at runtime anyway. */ - if (fpinfo->use_remote_estimate) - { - RangeTblEntry *rte = planner_rt_fetch(baserel->relid, root); - Oid userid = rte->checkAsUser ? rte->checkAsUser : GetUserId(); - - fpinfo->user = GetUserMapping(userid, fpinfo->server->serverid); - } - else - fpinfo->user = NULL; + rte = planner_rt_fetch(baserel->relid, root); + fpinfo->userid = rte->checkAsUser ? rte->checkAsUser : GetUserId(); So, wait a minute, remote estimates aren't optional any more? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers