> I personally don't see how this patch is 'ready for committer'. I realize > that that state is sometimes used to denote that review needs to be > "escalated", but it still seemspremature. > > Unless I miss something there hasn't been any API level review of this? > Also, aren't there several open items? > Even though some interface specifications are revised according to the comment from Tom on the last development cycle, the current set of interfaces are not reviewed by committers. I really want this.
Here are two open items that we want to wait for committers comments. * Whether set_cheapest() is called for all relkind? This pactch moved set_cheapest() to the end of set_rel_pathlist(), to consolidate entrypoint of custom-plan-provider handler function. It also implies CPP can provider alternative paths towards non-regular relations (like sub-queries, functions, ...). Hanada-san wonder whether we really have a case to run alternative sub-query code. Even though I don't have usecases for alternative sub-query execution logic, but we also don't have a reason why not to restrict it. * How argument of add_path handler shall be derivered? The handler function (that adds custom-path to the required relation scan if it can provide) is declared with an argument with INTERNAL data type. Extension needs to have type-cast on the supplied pointer to customScanArg data-type (or potentially customHashJoinArg and so on...) according to the custom plan class. I think it is well extendable design than strict argument definitions, but Hanada-san wonder whether it is the best design. > Perhaps there needs to be a stage between 'needs review' and 'ready for > committer'? > It needs clarification of 'ready for committer'. I think interface specification is a kind of task to be discussed with committers because preference/viewpoint of rr-reviewer are not always same opinion with them. Thanks, -- NEC OSS Promotion Center / PG-Strom Project KaiGai Kohei <kai...@ak.jp.nec.com> > -----Original Message----- > From: Andres Freund [mailto:and...@2ndquadrant.com] > Sent: Friday, July 18, 2014 3:12 AM > To: Shigeru Hanada > Cc: Kaigai Kouhei(海外 浩平); Kohei KaiGai; Simon Riggs; Tom Lane; Stephen > Frost; Robert Haas; PgHacker; Jim Mlodgenski; Peter Eisentraut > Subject: Re: [HACKERS] [v9.5] Custom Plan API > > On 2014-07-16 10:43:08 +0900, Shigeru Hanada wrote: > > Kaigai-san, > > > > 2014-07-15 21:37 GMT+09:00 Kouhei Kaigai <kai...@ak.jp.nec.com>: > > > Sorry, expected result of sanity-check test was not updated on > > > renaming to pg_custom_plan_provider. > > > The attached patch fixed up this point. > > > > I confirmed that all regression tests passed, so I marked the patch as > > "Ready for committer". > > I personally don't see how this patch is 'ready for committer'. I realize > that that state is sometimes used to denote that review needs to be > "escalated", but it still seemspremature. > > Unless I miss something there hasn't been any API level review of this? > Also, aren't there several open items? > > Perhaps there needs to be a stage between 'needs review' and 'ready for > committer'? > > Greetings, > > Andres Freund > > -- > Andres Freund http://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
