Re: Can we rely on the ordering of paths in pathlist?

On Tue, Apr 11, 2023 at 11:03 AM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:

> As the comment above add_path() says, 'The pathlist is kept sorted by
> total_cost, with cheaper paths at the front.' And it seems that
> get_cheapest_parallel_safe_total_inner() relies on this ordering
> (without being mentioned in the comments, which I think we should do).
>

I think the answer for ${subject} should be yes. Per the comments in
add_partial_path, we have

* add_partial_path
*
* As in add_path, the partial_pathlist is kept sorted with the cheapest
* total path in front. This is depended on by multiple places, which
* just take the front entry as the cheapest path without searching.
*
.

> I'm wondering if this is the right thing to do, as in other places
> cheapest total cost is found by compare_path_costs(), which would
> consider startup cost if paths have the same total cost, and that seems
> more sensible.
>
> Attach a trivial patch to make get_cheapest_parallel_safe_total_inner
> act this way.
>
>
Did you run into any real issue with the current coding? If we have to
"consider startup cost if paths have the same total cost", we still can
rely on the ordering and stop iterating when the total_cost becomes
bigger to avoid scanning all the paths in pathlist.

But if you are complaining the function prototype, where is the pathlist
may be not presorted, I think the better way maybe changes it from:
Path *get_cheapest_parallel_safe_total_inner(List *paths) to
Path *get_cheapest_parallel_safe_total_inner(RelOptInfo *rel);
and scan the rel->pathlist in the function body.

--
Best Regards
Andy Fan