Robert Haas <robertmhaas@gmail.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