"Imai, Yoshikazu" <imai.yoshik...@jp.fujitsu.com> writes: > I changed the state of this patch to "Ready for Committer".
I pushed this with a bit of extra tweaking --- notably, I added an Assert to ExecFindMatchingSubPlans to guard against the possibility that someone calls it when the remapping hasn't been done. I think that the main win here is the recognition that we only need remapping when both do_initial_prune and do_exec_prune are true. I suspect that's an unusual case, so that we'll usually get to skip this code altogether. I am not really convinced that the original idea of replacing the loop-over-subplans with a bms_next_member() loop is a good one. Yeah, it wins in the best case where there are a lot of subplans and initial pruning got rid of nearly all of them --- but how often is that true, given the context that both do_initial_prune and do_exec_prune are needed? And does it still win if most of the subplans *didn't* get pruned here? bms_next_member() is not cheap compared to an increment-and-compare. I am distressed by the fact that your performance testing seems to have examined only this best case. IMO performance testing should usually worry more about the possible downside in the worst case. Rather than arguing about that, though, I think we should focus on something else: I'd be a lot happier if this remapping logic just disappeared completely. I doubt it's a performance issue now that we run it only when initial and exec pruning are both needed. But it seems like a source of complexity and bugs, most especially the latter now that common cases won't run it. Why is it that we can't fix things so that ExecFindMatchingSubPlans just uses the original planner output data? regards, tom lane