Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-18 Thread Kouhei Kaigai
15 8:44 AM > To: Tom Lane; Kohei KaiGai > Cc: Robert Haas; Thom Brown; Shigeru Hanada; pgsql-hackers@postgreSQL.org > Subject: Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API) > > > A possible compromise that we could perhaps still wedge into 9.5 is to > &

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-15 Thread Shigeru Hanada
2015-05-15 8:43 GMT+09:00 Kouhei Kaigai : > Regarding of FDW, as Hanada-san mentioned, I'm uncertain whether > similar feature is also needed because its join-pushdown feature > scan on the result-set of remotely joined relations, thus no need > to have local child Path nodes. > So, I put this cust

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-14 Thread Kouhei Kaigai
> A possible compromise that we could perhaps still wedge into 9.5 is to > extend CustomPath with a List of child Paths, and CustomScan with a List > of child Plans, which createplan.c would know to build from the Paths, > and other modules would then also be aware of these children. I find that >

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-12 Thread Shigeru Hanada
2015-05-12 10:24 GMT+09:00 Kouhei Kaigai : > option-2) > Tom's suggestion. Add a new list field of Path nodes on CustomPath, > then create_customscan_plan() will call static create_plan_recurse() > function to construct child plan nodes. > Probably, the attached patch will be an image of this enhan

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-12 Thread Kouhei Kaigai
AM > To: 'Andres Freund'; 'Robert Haas' > Cc: 'Tom Lane'; 'Kohei KaiGai'; 'Thom Brown'; 'Shigeru Hanada'; > 'pgsql-hackers@postgreSQL.org' > Subject: RE: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-11 Thread Kouhei Kaigai
12:48 PM > To: 'Andres Freund'; Robert Haas > Cc: Tom Lane; Kohei KaiGai; Thom Brown; Shigeru Hanada; > pgsql-hackers@postgreSQL.org > Subject: RE: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API) > > > On 2015-05-10 21:26:26 -0400, Robert Haas w

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-11 Thread Robert Haas
On Sun, May 10, 2015 at 11:07 PM, Andres Freund wrote: > On 2015-05-10 22:51:33 -0400, Robert Haas wrote: >> > And there's definitely some things >> > around that pretty much only still exist because changing them would >> > break too much stuff. >> >> Such as what? > > Without even thinking about

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-10 Thread Kouhei Kaigai
> On 2015-05-10 21:26:26 -0400, Robert Haas wrote: > > On Sun, May 10, 2015 at 8:41 PM, Tom Lane wrote: > > > > This commit reverts create_plan_recurse() as static function. > > > Yes. I am not convinced that external callers should be calling that, > > > and would prefer not to enlarge createpla

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-10 Thread Andres Freund
On 2015-05-10 22:51:33 -0400, Robert Haas wrote: > > And there's definitely some things > > around that pretty much only still exist because changing them would > > break too much stuff. > > Such as what? Without even thinking about it: * linitial vs lfirst vs lnext. That thing still induces an i

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-10 Thread Robert Haas
On Sun, May 10, 2015 at 10:37 PM, Andres Freund wrote: > On 2015-05-10 21:53:45 -0400, Robert Haas wrote: >> Please name EVEN ONE instance in which core development has been >> prevented for fear of changing a function API. > > Even *moving* function declarations to a different file has been laudl

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-10 Thread Andres Freund
On 2015-05-10 21:53:45 -0400, Robert Haas wrote: > Please name EVEN ONE instance in which core development has been > prevented for fear of changing a function API. Even *moving* function declarations to a different file has been laudly and repeatedly complained about... And there's definitely som

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-10 Thread Andres Freund
On 2015-05-10 21:26:26 -0400, Robert Haas wrote: > On Sun, May 10, 2015 at 8:41 PM, Tom Lane wrote: > > > This commit reverts create_plan_recurse() as static function. > > Yes. I am not convinced that external callers should be calling that, > > and would prefer not to enlarge createplan.c's API

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-10 Thread Robert Haas
On Sun, May 10, 2015 at 9:34 PM, Tom Lane wrote: > Robert Haas writes: >> Your unwillingness to make functions global or to stick PGDLLIMPORT >> markings on variables that people want access to is hugely >> handicapping extension authors. Many people have complained about >> that on multiple occ

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-10 Thread Tom Lane
Robert Haas writes: > Your unwillingness to make functions global or to stick PGDLLIMPORT > markings on variables that people want access to is hugely > handicapping extension authors. Many people have complained about > that on multiple occasions. Frankly, I find it obstructionist and > petty.

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-10 Thread Robert Haas
On Sun, May 10, 2015 at 8:41 PM, Tom Lane wrote: > Kohei KaiGai writes: >> I briefly checked your updates. >> Even though it is not described in the commit-log, I noticed a >> problematic change. > >> This commit reverts create_plan_recurse() as static function. > > Yes. I am not convinced that

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-10 Thread Kouhei Kaigai
> Kohei KaiGai writes: > > I briefly checked your updates. > > Even though it is not described in the commit-log, I noticed a > > problematic change. > > > This commit reverts create_plan_recurse() as static function. > > Yes. I am not convinced that external callers should be calling that, > a

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-10 Thread Tom Lane
Kohei KaiGai writes: > I briefly checked your updates. > Even though it is not described in the commit-log, I noticed a > problematic change. > This commit reverts create_plan_recurse() as static function. Yes. I am not convinced that external callers should be calling that, and would prefer no

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-10 Thread Kohei KaiGai
Tom, I briefly checked your updates. Even though it is not described in the commit-log, I noticed a problematic change. This commit reverts create_plan_recurse() as static function. It means extension cannot have child node, even if it wants to add a custom-join logic. Please assume a simple case

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-10 Thread Tom Lane
I've committed a cleanup patch along the lines discussed. One thought is that we could test the nondefault-scan-tuple logic without a lot of work by modifying the way postgres_fdw deals with columns it decides don't need to be fetched. Right now it injects NULL columns so that the remote query re

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-10 Thread Tom Lane
Robert Haas writes: > On Sat, May 9, 2015 at 1:05 PM, Tom Lane wrote: >> For these reasons, I think that if an FDW tried to be laxer than "tables >> must be on the same pg_foreign_server entry to be joined", the odds >> approach unity that it would be broken, and probably dangerously broken. >> S

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-10 Thread Robert Haas
On Sat, May 9, 2015 at 1:05 PM, Tom Lane wrote: >> I originally wanted to go quite the other way with this and check for >> join pushdown via handler X any time at least one of the two relations >> involved used handler X, even if the other one used some other handler >> or was a plain table. In

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-09 Thread Tom Lane
Robert Haas writes: > On Fri, May 8, 2015 at 5:48 PM, Tom Lane wrote: >> I think that we'd really be better off insisting on same server (as in >> same pg_foreign_server OID), hence automatically same FDW, and what's >> even more important, same user mapping for any possible query execution >> co

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-09 Thread Kohei KaiGai
2015-05-09 8:18 GMT+09:00 Kohei KaiGai : > 2015-05-09 2:46 GMT+09:00 Tom Lane : >> Kouhei Kaigai writes: I've been trying to code-review this patch, because the documentation seemed several bricks shy of a load, and I find myself entirely confused by the fdw_ps_tlist and custom_ps_t

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-09 Thread Kohei KaiGai
2015-05-09 8:32 GMT+09:00 Kohei KaiGai : > 2015-05-09 3:51 GMT+09:00 Tom Lane : >> Robert Haas writes: >>> On Fri, May 8, 2015 at 1:46 PM, Tom Lane wrote: That's nice, but 9.5 feature freeze is only a week away. I don't have a lot of confidence that this stuff is actually in a state wh

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-09 Thread Kohei KaiGai
2015-05-09 11:21 GMT+09:00 Robert Haas : > On Fri, May 8, 2015 at 5:48 PM, Tom Lane wrote: >> ... btw, I just noticed something that had escaped me because it seems so >> obviously wrong that I had not even stopped to consider the possibility >> that the code was doing what it's doing. To wit, th

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-08 Thread Robert Haas
On Fri, May 8, 2015 at 5:48 PM, Tom Lane wrote: > ... btw, I just noticed something that had escaped me because it seems so > obviously wrong that I had not even stopped to consider the possibility > that the code was doing what it's doing. To wit, that the planner > supposes that two foreign tab

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-08 Thread Robert Haas
On Fri, May 8, 2015 at 2:51 PM, Tom Lane wrote: > Well, we have two alternatives. I can keep hacking on this and get it > to a state where it seems credible to me, but we won't have any proof > that it actually works (though perhaps we could treat any problems > as bugs that should hopefully get

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-08 Thread Kohei KaiGai
2015-05-09 3:51 GMT+09:00 Tom Lane : > Robert Haas writes: >> On Fri, May 8, 2015 at 1:46 PM, Tom Lane wrote: >>> That's nice, but 9.5 feature freeze is only a week away. I don't have a >>> lot of confidence that this stuff is actually in a state where we won't >>> regret shipping it in 9.5. > >

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-08 Thread Kohei KaiGai
2015-05-09 2:46 GMT+09:00 Tom Lane : > Kouhei Kaigai writes: >>> I've been trying to code-review this patch, because the documentation >>> seemed several bricks shy of a load, and I find myself entirely confused >>> by the fdw_ps_tlist and custom_ps_tlist fields. > >> Main-point of your concern is

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-08 Thread Kohei KaiGai
2015-05-09 6:48 GMT+09:00 Tom Lane : > ... btw, I just noticed something that had escaped me because it seems so > obviously wrong that I had not even stopped to consider the possibility > that the code was doing what it's doing. To wit, that the planner > supposes that two foreign tables are pote

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-08 Thread Tom Lane
... btw, I just noticed something that had escaped me because it seems so obviously wrong that I had not even stopped to consider the possibility that the code was doing what it's doing. To wit, that the planner supposes that two foreign tables are potentially remote-joinable if they share the sam

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-08 Thread Tom Lane
Robert Haas writes: > On Fri, May 8, 2015 at 1:46 PM, Tom Lane wrote: >> That's nice, but 9.5 feature freeze is only a week away. I don't have a >> lot of confidence that this stuff is actually in a state where we won't >> regret shipping it in 9.5. > Yeah. The POC you were asking for upthread

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-08 Thread Robert Haas
On Fri, May 8, 2015 at 1:46 PM, Tom Lane wrote: > That's nice, but 9.5 feature freeze is only a week away. I don't have a > lot of confidence that this stuff is actually in a state where we won't > regret shipping it in 9.5. Yeah. The POC you were asking for upthread certainly exists and has fo

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-08 Thread Tom Lane
Kouhei Kaigai writes: >> I've been trying to code-review this patch, because the documentation >> seemed several bricks shy of a load, and I find myself entirely confused >> by the fdw_ps_tlist and custom_ps_tlist fields. > Main-point of your concern is lack of documentation/comments to introduce

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-07 Thread Kouhei Kaigai
> Robert Haas writes: > > On Thu, Apr 30, 2015 at 5:21 PM, Kouhei Kaigai wrote: > >> I wanted to submit the v14 after the above items get clarified. > >> The attached patch (v14) includes all what you suggested in the previous > >> message. > > > Committed, after heavily working over the documen

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-07 Thread Tom Lane
Robert Haas writes: > On Thu, Apr 30, 2015 at 5:21 PM, Kouhei Kaigai wrote: >> I wanted to submit the v14 after the above items get clarified. >> The attached patch (v14) includes all what you suggested in the previous >> message. > Committed, after heavily working over the documentation, and wi

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-05-01 Thread Robert Haas
On Thu, Apr 30, 2015 at 5:21 PM, Kouhei Kaigai wrote: > I wanted to submit the v14 after the above items get clarified. > The attached patch (v14) includes all what you suggested in the previous > message. Committed, after heavily working over the documentation, and with some more revisions to th

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-30 Thread Kouhei Kaigai
> On Thu, Apr 30, 2015 at 9:16 AM, Kouhei Kaigai wrote: > > It seems to me the code block for T_ForeignScan and T_CustomScan in > > setrefs.c are a bit large. It may be better to have a separate > > function like T_IndexOnlyScan. > > How about your opinion? > > Either way is OK with me. Please d

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-30 Thread Robert Haas
On Thu, Apr 30, 2015 at 9:16 AM, Kouhei Kaigai wrote: > It seems to me the code block for T_ForeignScan and T_CustomScan in > setrefs.c are a bit large. It may be better to have a separate > function like T_IndexOnlyScan. > How about your opinion? Either way is OK with me. Please do as you think

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-30 Thread Kouhei Kaigai
> On Sun, Apr 26, 2015 at 10:00 PM, Kouhei Kaigai wrote: > > The attached patch v13 is revised one according to the suggestion > > by Robert. > > Thanks. > > The last hunk in foreign.c is a useless whitespace change. > Sorry, my oversight. > + /* actually, not shift members */ > > Change

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-29 Thread Robert Haas
On Sun, Apr 26, 2015 at 10:00 PM, Kouhei Kaigai wrote: > The attached patch v13 is revised one according to the suggestion > by Robert. Thanks. The last hunk in foreign.c is a useless whitespace change. + /* actually, not shift members */ Change to: "shift of 0 is the same as copying" B

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-29 Thread Robert Haas
On Mon, Apr 27, 2015 at 5:05 AM, Shigeru HANADA wrote: > Currently INNER JOINs with unsafe join conditions are not pushed down, so > such test is not in the suit. As you say, in theory, INNER JOINs can be > pushed down even they have push-down-unsafe join conditions, because such > conditions

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-28 Thread Ashutosh Bapat
On Fri, Apr 24, 2015 at 3:08 PM, Shigeru HANADA wrote: > Hi Ashutosh, > > Thanks for the review. > > 2015/04/22 19:28、Ashutosh Bapat のメール: > > Tests > > --- > > 1.The postgres_fdw test is re/setting enable_mergejoin at various > places. The goal of these tests seems to be to test the sanity

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-27 Thread Shigeru HANADA
Hi Ashutosh, Thanks for the review. 2015/04/22 19:28、Ashutosh Bapat のメール: > Tests > --- > 1.The postgres_fdw test is re/setting enable_mergejoin at various places. The > goal of these tests seems to be to test the sanity of foreign plans > generated. So, it might be better to reset enable_

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-26 Thread Kouhei Kaigai
015 11:23 PM > To: 'Robert Haas' > Cc: Tom Lane; Thom Brown; Shigeru Hanada; pgsql-hackers@postgreSQL.org > Subject: Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API) > > > On Wed, Apr 22, 2015 at 10:48 PM, Kouhei Kaigai > >

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-24 Thread Kouhei Kaigai
> On Wed, Apr 22, 2015 at 10:48 PM, Kouhei Kaigai wrote: > >> + else if (scan->scanrelid == 0 && > >> +(IsA(scan, ForeignScan) || IsA(scan, CustomScan))) > >> + varno = INDEX_VAR; > >> > >> Suppose scan->scanrelid == 0 but the scan type is something else

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-24 Thread Robert Haas
On Wed, Apr 22, 2015 at 10:48 PM, Kouhei Kaigai wrote: >> + else if (scan->scanrelid == 0 && >> +(IsA(scan, ForeignScan) || IsA(scan, CustomScan))) >> + varno = INDEX_VAR; >> >> Suppose scan->scanrelid == 0 but the scan type is something else? Is >> tha

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-24 Thread Shigeru HANADA
Hi Ashutosh, Thanks for the review. 2015/04/22 19:28、Ashutosh Bapat のメール: > Tests > --- > 1.The postgres_fdw test is re/setting enable_mergejoin at various places. The > goal of these tests seems to be to test the sanity of foreign plans > generated. So, it might be better to reset enable_

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-22 Thread Kouhei Kaigai
Hi Robert, Thanks for your comments. > A few random cosmetic problems: > > - The hunk in allpaths.c is useless. > - The first hunk in fdwapi.h contains an extra space before the > closing parenthesis. > OK, it's my oversight. > And then: > > + else if (scan->scanrelid == 0 && > +

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-22 Thread Robert Haas
On Fri, Mar 13, 2015 at 8:02 PM, Tom Lane wrote: > Robert Haas writes: >> On Fri, Mar 13, 2015 at 2:31 PM, Tom Lane wrote: >>> I don't object to the concept, but I think that is a pretty bad place >>> to put the hook call: add_paths_to_joinrel is typically called multiple >>> (perhaps *many*) ti

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-22 Thread Robert Haas
On Tue, Apr 21, 2015 at 10:33 PM, Kouhei Kaigai wrote: > [ new patch ] A little more nitpicking: ExecInitForeignScan() and ExecInitCustomScan() could declare currentRelation inside the if (scanrelid > 0) block instead of in the outer scope. I'm not too excited about the addition of GetFdwHandle

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-22 Thread Robert Haas
On Wed, Mar 25, 2015 at 9:51 PM, Kouhei Kaigai wrote: > The attached patch adds GetForeignJoinPaths call on make_join_rel() only when > 'joinrel' is actually built and both of child relations are managed by same > FDW driver, prior to any other built-in join paths. > I adjusted the hook definition

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-21 Thread Kouhei Kaigai
Hanada-san, > I reviewed the Custom/Foreign join API patch again after writing a patch of > join > push-down support for postgres_fdw. > Thanks for your dedicated jobs, my comments are inline below. > Here, please let me summarize the changes in the patch as the result of my > review. > > * Ad

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-20 Thread Shigeru HANADA
Kaigai-san, I reviewed the Custom/Foreign join API patch again after writing a patch of join push-down support for postgres_fdw. 2015/03/26 10:51、Kouhei Kaigai のメール: >>> Or bottom of make_join_rel(). IMO build_join_rel() is responsible for just >> building (or searching from a list) a RelOptI

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-17 Thread Kouhei Kaigai
HANADA [mailto:shigeru.han...@gmail.com] > Sent: Friday, April 17, 2015 1:44 PM > To: Kaigai Kouhei(海外 浩平) > Cc: Ashutosh Bapat; Robert Haas; Tom Lane; Thom Brown; > pgsql-hackers@postgreSQL.org > Subject: ##freemail## Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] > Custom >

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-16 Thread Kouhei Kaigai
KaiGai Kohei > -Original Message- > From: Shigeru HANADA [mailto:shigeru.han...@gmail.com] > Sent: Thursday, April 16, 2015 5:06 PM > To: Kaigai Kouhei(海外 浩平) > Cc: Ashutosh Bapat; Robert Haas; Tom Lane; Thom Brown; > pgsql-hackers@postgreSQL.org > Subject: ##freemail##

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-15 Thread Kouhei Kaigai
on why to split this feature. Thanks, -- NEC Business Creation Division / PG-Strom Project KaiGai Kohei > -Original Message- > From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Shigeru HANADA > Sent: Tuesday, April 14, 2015 7:

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-14 Thread Shigeru HANADA
KaiGai-san, 2015/04/14 14:04、Kouhei Kaigai のメール: > >> * Fix typos >> >> Please review the v11 patch, and mark it as “ready for committer” if it’s ok. >> > It's OK for me, and wants to be reviewed by other people to get it committed. > Thanks! >> In addition to essential features, I tried to

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-13 Thread Kouhei Kaigai
Hanada-san, > Thanks for further review, but I found two bugs in v10 patch. > I’ve fixed them and wrapped up v11 patch here. > > * Fix bug about illegal column order > > Scan against a base relation returns columns in order of column definition, > but > its target list might require different o

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-09 Thread Kouhei Kaigai
> 2015/04/09 10:48、Kouhei Kaigai のメール: > * merge_fpinfo() > >>> It seems to me fpinfo->rows should be joinrel->rows, and > >>> fpinfo->width also should be joinrel->width. > >>> No need to have special intelligence here, isn't it? > >> > >> > >> Oops. They are vestige of my struggle which disabled

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-08 Thread Kouhei Kaigai
Hanada-san, > In addition to your comments, I removed useless code that retrieves > ForeignPath > from outer/inner RelOptInfo and store them into ForeignPath#fdw_private. Now > postgres_fdw’s join pushd-down is free from existence of ForeignPath under the > join relation. This means that we can

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-06 Thread Kouhei Kaigai
t; -Original Message- > From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Shigeru Hanada > Sent: Friday, April 03, 2015 7:32 PM > To: Kaigai Kouhei(海外 浩平) > Cc: Ashutosh Bapat; Robert Haas; Tom Lane; Thom Brown; >

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-25 Thread Shigeru HANADA
2015/03/26 10:51、Kouhei Kaigai のメール: > The attached patch adds GetForeignJoinPaths call on make_join_rel() only when > 'joinrel' is actually built and both of child relations are managed by same > FDW driver, prior to any other built-in join paths. > I adjusted the hook definition a little bit, b

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-25 Thread Kouhei Kaigai
> > Or bottom of make_join_rel(). IMO build_join_rel() is responsible for just > building (or searching from a list) a RelOptInfo for given relids. After that > make_join_rel() calls add_paths_to_joinrel() with appropriate arguments per > join > type to generate actual Paths implements the join.

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-25 Thread Kouhei Kaigai
> 2015/03/25 19:47、Kouhei Kaigai のメール: > > The reason why FDW handler was called multiple times on your example is, > > your modified make_join_rel() does not check whether build_join_rel() > > actually build a new RelOptInfo, or just a cache reference, doesn't it? > > > > Yep. After that change

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-25 Thread Shigeru HANADA
2015/03/25 19:47、Kouhei Kaigai のメール: > The reason why FDW handler was called multiple times on your example is, > your modified make_join_rel() does not check whether build_join_rel() > actually build a new RelOptInfo, or just a cache reference, doesn't it? > Yep. After that change calling cou

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-25 Thread Shigeru HANADA
2015/03/25 18:53、Ashutosh Bapat のメール: > > > On Wed, Mar 25, 2015 at 3:14 PM, Shigeru HANADA > wrote: > > Or bottom of make_join_rel(). IMO build_join_rel() is responsible for just > building (or searching from a list) a RelOptInfo for given relids. After > that make_join_rel() calls add

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-25 Thread Kouhei Kaigai
> 2015/03/25 19:09、Kouhei Kaigai のメール: > > >> On Wed, Mar 25, 2015 at 3:14 PM, Shigeru HANADA > wrote: > >>Or bottom of make_join_rel(). IMO build_join_rel() is responsible for > >> just building (or searching from a list) a RelOptInfo for given relids. > >> After > >> that make_join_rel(

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-25 Thread Kouhei Kaigai
> 2015/03/25 12:59、Kouhei Kaigai のメール: > > >>> At this moment, I'm not 100% certain about its logic. Especially, I didn't > >>> test SEMI- and ANTI- join cases yet. > >>> However, time is money - I want people to check overall design first, > >>> rather > >>> than detailed debugging. Please tell

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-25 Thread Shigeru HANADA
2015/03/25 19:09、Kouhei Kaigai のメール: >> On Wed, Mar 25, 2015 at 3:14 PM, Shigeru HANADA >> wrote: >> Or bottom of make_join_rel(). IMO build_join_rel() is responsible for >> just building (or searching from a list) a RelOptInfo for given relids. >> After >> that make_join_rel() calls a

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-25 Thread Kouhei Kaigai
> On Wed, Mar 25, 2015 at 3:14 PM, Shigeru HANADA > wrote: > Or bottom of make_join_rel(). IMO build_join_rel() is responsible for > just building (or searching from a list) a RelOptInfo for given relids. After > that make_join_rel() calls add_paths_to_joinrel() with appropriate arguments

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-25 Thread Ashutosh Bapat
On Wed, Mar 25, 2015 at 3:14 PM, Shigeru HANADA wrote: > > Or bottom of make_join_rel(). IMO build_join_rel() is responsible for > just building (or searching from a list) a RelOptInfo for given relids. > After that make_join_rel() calls add_paths_to_joinrel() with appropriate > arguments per jo

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-25 Thread Shigeru HANADA
2015/03/25 12:59、Kouhei Kaigai のメール: >>> At this moment, I'm not 100% certain about its logic. Especially, I didn't >>> test SEMI- and ANTI- join cases yet. >>> However, time is money - I want people to check overall design first, rather >>> than detailed debugging. Please tell me if I misunders

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-24 Thread Kouhei Kaigai
eru.han...@gmail.com] > Sent: Tuesday, March 24, 2015 7:36 PM > To: Kaigai Kouhei(海外 浩平) > Cc: Robert Haas; Tom Lane; Ashutosh Bapat; Thom Brown; > pgsql-hackers@postgreSQL.org > Subject: Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API) > > 2015/03/23

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-24 Thread Shigeru HANADA
2015/03/23 9:12、Kouhei Kaigai のメール: > Sorry for my response late. It was not easy to code during business trip. > > The attached patch adds a hook for FDW/CSP to replace entire join-subtree > by a foreign/custom-scan, according to the discussion upthread. > > GetForeignJoinPaths handler of FDW

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-22 Thread Kouhei Kaigai
> On Wed, Mar 18, 2015 at 9:33 AM, Kouhei Kaigai wrote: > >> On Wed, Mar 18, 2015 at 2:34 AM, Kouhei Kaigai > >> wrote: > >> > So, overall consensus for the FDW hook location is just before the > >> > set_chepest() > >> > at standard_join_search() and merge_clump(), isn't it? > >> > >> Yes, I t

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-18 Thread Robert Haas
On Wed, Mar 18, 2015 at 9:33 AM, Kouhei Kaigai wrote: >> On Wed, Mar 18, 2015 at 2:34 AM, Kouhei Kaigai wrote: >> > So, overall consensus for the FDW hook location is just before the >> > set_chepest() >> > at standard_join_search() and merge_clump(), isn't it? >> >> Yes, I think so. >> >> > Let

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-18 Thread Kouhei Kaigai
> On Wed, Mar 18, 2015 at 2:34 AM, Kouhei Kaigai wrote: > > So, overall consensus for the FDW hook location is just before the > > set_chepest() > > at standard_join_search() and merge_clump(), isn't it? > > Yes, I think so. > > > Let me make a design of FDW hook to reduce code duplications for

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-18 Thread Robert Haas
On Wed, Mar 18, 2015 at 2:34 AM, Kouhei Kaigai wrote: > So, overall consensus for the FDW hook location is just before the > set_chepest() > at standard_join_search() and merge_clump(), isn't it? Yes, I think so. > Let me make a design of FDW hook to reduce code duplications for each FDW > dri

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-17 Thread Kouhei Kaigai
> Robert Haas writes: > > On Tue, Mar 17, 2015 at 10:11 AM, Kouhei Kaigai > > wrote: > >> It might be an idea if foreign-scan path is not wiped out regardless of the > >> estimated cost, we will be able to construct an entirely remote-join path > >> even if intermediation path is expensive than

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-17 Thread Tom Lane
Robert Haas writes: > On Tue, Mar 17, 2015 at 10:11 AM, Kouhei Kaigai wrote: >> It might be an idea if foreign-scan path is not wiped out regardless of the >> estimated cost, we will be able to construct an entirely remote-join path >> even if intermediation path is expensive than local join. >>

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-17 Thread Ashutosh Bapat
On Tue, Mar 17, 2015 at 8:34 PM, Robert Haas wrote: > On Tue, Mar 17, 2015 at 10:11 AM, Kouhei Kaigai > wrote: > >> A way to work around this is to leave the ForeignPaths (there can > possibly be > >> only one foreign path per join relation) in the joinrel without > removing them. > >> FDW shoul

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-17 Thread Robert Haas
On Tue, Mar 17, 2015 at 10:11 AM, Kouhei Kaigai wrote: >> A way to work around this is to leave the ForeignPaths (there can possibly be >> only one foreign path per join relation) in the joinrel without removing >> them. >> FDW should work on joining two relations if they have foreign paths in th

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-17 Thread Kouhei Kaigai
> On Sat, Mar 14, 2015 at 3:48 AM, Robert Haas wrote: > > > On Fri, Mar 13, 2015 at 2:31 PM, Tom Lane wrote: > > Robert Haas writes: > >> Another bit of this that I think we could commit without fretting > >> about it too much is the code adding set_join_pathlist_hook.

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-17 Thread Ashutosh Bapat
On Tue, Mar 17, 2015 at 10:28 AM, Shigeru Hanada wrote: > 2015-03-14 7:18 GMT+09:00 Robert Haas : > > I think the foreign data wrapper join pushdown case, which also aims > > to substitute a scan for a join, is interesting to think about, even > > though it's likely to be handled by a new FDW met

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-17 Thread Ashutosh Bapat
On Sat, Mar 14, 2015 at 3:48 AM, Robert Haas wrote: > On Fri, Mar 13, 2015 at 2:31 PM, Tom Lane wrote: > > Robert Haas writes: > >> Another bit of this that I think we could commit without fretting > >> about it too much is the code adding set_join_pathlist_hook. This is > >> - I think - analo

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-17 Thread Etsuro Fujita
On 2015/03/14 7:18, Robert Haas wrote: I think the foreign data wrapper join pushdown case, which also aims to substitute a scan for a join, is interesting to think about, even though it's likely to be handled by a new FDW method instead of via the hook. Where should the FDW method get called fr

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-16 Thread Kouhei Kaigai
> -Original Message- > From: Shigeru Hanada [mailto:shigeru.han...@gmail.com] > Sent: Monday, March 16, 2015 9:59 PM > To: Robert Haas > Cc: Tom Lane; Thom Brown; Kaigai Kouhei(海外 浩平); pgsql-hackers@postgreSQL.org > Subject: ##freemail## Re: Custom/Foreign-Join-APIs (R

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-16 Thread Shigeru Hanada
2015-03-14 7:18 GMT+09:00 Robert Haas : > I think the foreign data wrapper join pushdown case, which also aims > to substitute a scan for a join, is interesting to think about, even > though it's likely to be handled by a new FDW method instead of via > the hook. Where should the FDW method get ca

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-16 Thread Robert Haas
On Sat, Mar 14, 2015 at 10:37 PM, Kouhei Kaigai wrote: > From the standpoint of extension development, I'm uncertain whether we > can easily reproduce information needed to compute alternative paths on > the hook at standard_join_search(), like a hook at add_paths_to_joinrel(). > > (Please correct

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-15 Thread Kouhei Kaigai
age- > From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Kouhei Kaigai > Sent: Sunday, March 15, 2015 11:38 AM > To: Robert Haas; Tom Lane > Cc: Thom Brown; Shigeru Hanada; pgsql-hackers@postgreSQL.org > Subject: Re: Custom/Foreign-Jo

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-14 Thread Kouhei Kaigai
> On Fri, Mar 13, 2015 at 2:31 PM, Tom Lane wrote: > > Robert Haas writes: > >> Another bit of this that I think we could commit without fretting > >> about it too much is the code adding set_join_pathlist_hook. This is > >> - I think - analogous to set_rel_pathlist_hook, and like that hook, > >

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-13 Thread Tom Lane
Robert Haas writes: > On Fri, Mar 13, 2015 at 2:31 PM, Tom Lane wrote: >> I don't object to the concept, but I think that is a pretty bad place >> to put the hook call: add_paths_to_joinrel is typically called multiple >> (perhaps *many*) times per joinrel and thus this placement would force >> a

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-13 Thread Robert Haas
On Fri, Mar 13, 2015 at 2:31 PM, Tom Lane wrote: > Robert Haas writes: >> Another bit of this that I think we could commit without fretting >> about it too much is the code adding set_join_pathlist_hook. This is >> - I think - analogous to set_rel_pathlist_hook, and like that hook, >> could be u

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-13 Thread Tom Lane
Robert Haas writes: > Another bit of this that I think we could commit without fretting > about it too much is the code adding set_join_pathlist_hook. This is > - I think - analogous to set_rel_pathlist_hook, and like that hook, > could be used for other purposes than custom plan generation - e.g

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-13 Thread Robert Haas
On Thu, Mar 12, 2015 at 8:09 PM, Thom Brown wrote: > On 12 March 2015 at 21:28, Tom Lane wrote: >> Robert Haas writes: >>> I took a look at this patch today and noticed that it incorporates not >>> only documentation for the new functionality it adds, but also for the >>> custom-scan functionali

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-12 Thread Kouhei Kaigai
> On Mon, Mar 9, 2015 at 11:18 PM, Kouhei Kaigai wrote: > > The attached patch integrates a suggestion from Ashutosh Bapat. > > It allows to track set of relations involved in a join, but > > replaced by foreign-/custom-scan. It enables to make correct > > EXPLAIN output, if FDW/CSP driver makes h

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-12 Thread Thom Brown
On 12 March 2015 at 21:28, Tom Lane wrote: > Robert Haas writes: >> I took a look at this patch today and noticed that it incorporates not >> only documentation for the new functionality it adds, but also for the >> custom-scan functionality whose documentation I previously excluded >> from commi

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-12 Thread Tom Lane
Robert Haas writes: > I took a look at this patch today and noticed that it incorporates not > only documentation for the new functionality it adds, but also for the > custom-scan functionality whose documentation I previously excluded > from commit on the grounds that it needed more work, especia

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-03-12 Thread Robert Haas
On Mon, Mar 9, 2015 at 11:18 PM, Kouhei Kaigai wrote: > The attached patch integrates a suggestion from Ashutosh Bapat. > It allows to track set of relations involved in a join, but > replaced by foreign-/custom-scan. It enables to make correct > EXPLAIN output, if FDW/CSP driver makes human reada

  1   2   >