> The check for parallel_safe should be even cheaper than cost comparison > so I think it's better to do that first. The attached patch does this > and also updates the comment to mention the requirement about being > parallel-safe.
The patch was marked as "Needs review" so I decided to take a look.
Thanks for the review!
I see the reasoning behind the proposed change, but I'm not convinced that there will be any measurable performance improvements. Firstly, compare_path_costs() is rather cheap. Secondly, require_parallel_safe is `false` in most of the cases. Last but not least, one should prove that this particular place is a bottleneck under given loads. I doubt it is. Most of the time it's a network, disk I/O or locks.
So unless the author can provide benchmarks that show measurable benefits of the change I suggest rejecting it.
Hmm, I doubt that there would be any measurable performance gains from this minor tweak. I think this tweak is more about being cosmetic. But I'm OK if it is deemed unnecessary and thus rejected.
Another change in this patch is to mention the requirement about being parallel-safe in the comment.
* Find the cheapest path (according to the specified criterion) that - * satisfies the given pathkeys and parameterization. + * satisfies the given pathkeys and parameterization, and is parallel-safe + * if required.
