From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com> |
Cc: | Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Thom Brown <thom(at)linux(dot)com>, "pgsql-hackers(at)postgreSQL(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs) |
Date: | 2015-05-01 13:35:09 |
Message-ID: | CA+TgmoYvXU=vNLCKccg3=Z6LYV_0Atpk7jx_qrvtdR0Cmj3Hfg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Apr 27, 2015 at 5:07 AM, Shigeru Hanada
<shigeru(dot)hanada(at)gmail(dot)com> wrote:
> 2015-04-27 11:00 GMT+09:00 Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2015-05-01 13:38:53 | Re: proposal: disallow operator "=>" and use it for named parameters |
Previous Message | Petr Jelinek | 2015-05-01 13:29:07 | Re: proposal: disallow operator "=>" and use it for named parameters |