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

Robert Haas <robertmhaas(at)gmail(dot)com> 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. 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.

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
any user of the hook to do a lot of repetitive work.

I think the right placement is just before the set_cheapest call for
each joinrel, just as we did with set_rel_pathlist_hook. It looks
like those calls are at:

allpaths.c:1649 (in standard_join_search)
geqo_eval.c:270 (in merge_clump)

There are a couple of other set_cheapest calls that probably don't need
hooked, since they are for dummy (proven empty) rels, and it's not clear
how a hook could improve on an empty plan.

Also, this would leave you with a much shorter parameter list ;-) ...
really no reason to pass more than root and rel.

regards, tom lane