On Thu, Mar 12, 2015 at 8:09 PM, Thom Brown <t...@linux.com> wrote: > On 12 March 2015 at 21:28, Tom Lane <t...@sss.pgh.pa.us> wrote: >> Robert Haas <robertmh...@gmail.com> 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, especially to >>> improve the English. That decision was not popular at the time, and I >>> think we need to remedy it before going further with this. I had >>> hoped that someone else would care about this work enough to help with >>> the documentation, but it seems not, so today I went through the >>> documentation in this patch, excluded all of the stuff specific to >>> custom joins, and heavily edited the rest. The result is attached. >> >> Looks good; I noticed one trivial typo --- please s/returns/return/ under >> ExecCustomScan. Also, perhaps instead of just "set ps_ResultTupleSlot" >> that should say "fill ps_ResultTupleSlot with the next tuple in the >> current scan direction". > > Also: > > s/initalization/initialization/
Thanks to both of you for the review. I have committed it with those improvements. Please let me know if you spot anything else. 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. to delete paths we do not want to use. I've extracted this portion of the patch and adjusted the comments; if there are no objections, I will commit this bit also. Kaigai, note that your patch puts this hook and the call to GetForeignJoinPaths() in the wrong order. As in the baserel case, the hook should get the last word, after any FDW-specific handlers have been called, so that it can delete or modify paths as well as add them. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index 1da953f..2872430 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -21,6 +21,8 @@ #include "optimizer/pathnode.h" #include "optimizer/paths.h" +/* Hook for plugins to get control in add_paths_to_joinrel() */ +set_join_pathlist_hook_type set_join_pathlist_hook = NULL; #define PATH_PARAM_BY_REL(path, rel) \ ((path)->param_info && bms_overlap(PATH_REQ_OUTER(path), (rel)->relids)) @@ -260,6 +262,17 @@ add_paths_to_joinrel(PlannerInfo *root, restrictlist, jointype, sjinfo, &semifactors, param_source_rels, extra_lateral_rels); + + /* + * Allow a plugin to editorialize on the set of Paths for this join + * relation. It could add new paths by calling add_path(), or delete + * or modify paths added by the core code. + */ + if (set_join_pathlist_hook) + set_join_pathlist_hook(root, joinrel, outerrel, innerrel, + restrictlist, jointype, + sjinfo, &semifactors, + param_source_rels, extra_lateral_rels); } /* diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h index 6cad92e..c42c69d 100644 --- a/src/include/optimizer/paths.h +++ b/src/include/optimizer/paths.h @@ -30,6 +30,19 @@ typedef void (*set_rel_pathlist_hook_type) (PlannerInfo *root, RangeTblEntry *rte); extern PGDLLIMPORT set_rel_pathlist_hook_type set_rel_pathlist_hook; +/* Hook for plugins to get control in add_paths_to_joinrel() */ +typedef void (*set_join_pathlist_hook_type) (PlannerInfo *root, + RelOptInfo *joinrel, + RelOptInfo *outerrel, + RelOptInfo *innerrel, + List *restrictlist, + JoinType jointype, + SpecialJoinInfo *sjinfo, + SemiAntiJoinFactors *semifactors, + Relids param_source_rels, + Relids extra_lateral_rels); +extern PGDLLIMPORT set_join_pathlist_hook_type set_join_pathlist_hook; + /* Hook for plugins to replace standard_join_search() */ typedef RelOptInfo *(*join_search_hook_type) (PlannerInfo *root, int levels_needed,
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
