David Rowley <dgrowle...@gmail.com> writes: > Do you feel that the choice to create_plan() on the subplan before > planning the outer query is still a good one? ISTM that that was > required when the AlternativeSubplan decision was made during > execution, since we, of course, need a plan to execute. If the > decision is now being made in the planner then is it not better to > delay the create_plan() until later in planning?
Hm. That's well outside the scope I had in mind for this patch. In principle, you're right that we could postpone final planning of the subquery till later; but I fear it'd require quite a lot of refactoring to make it work that way. There's a lot of rather subtle timing dependencies in the processing done by createplan.c and setrefs.c, so I think this might be a lot more painful than it seems at first glance. And we'd only gain anything in cases that use AlternativeSubPlan, which is a minority of subplans, so on the whole I rather doubt it's worth the trouble. One inefficiency I see that we could probably get rid of is where make_subplan() is doing /* Now we can check if it'll fit in hash_mem */ /* XXX can we check this at the Path stage? */ if (subplan_is_hashable(plan)) { The only inputs subplan_is_hashable needs are the predicted rowcount and output width, which surely we could get from the Path. So we could save doing create_plan() when we decide the subquery output is too big to hash. OTOH, that's probably a pretty small minority of use-cases, so it might not be worth troubling over. regards, tom lane