public inbox for [email protected]  
help / color / mirror / Atom feed
From: wenhui qiu <[email protected]>
To: Richard Guo <[email protected]>
Cc: Andy Fan <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Can we rely on the ordering of paths in pathlist?
Date: Thu, 25 Jul 2024 16:18:49 +0800
Message-ID: <CAGjGUAKiE5kmtpp2A--9RPFnGtUtooFAQ0rxZ33QzA0Hn3pGqw@mail.gmail.com> (raw)
In-Reply-To: <CAMbWs48wLYmDZB47ZguQg6gncGkD_y6VWNtO_2=kppOb08dDOg@mail.gmail.com>
References: <CAMbWs49xR_2T1BPDw+vO4Bs7pqYiQn4aNaWdmA9BdVEaHhtktg@mail.gmail.com>
	<CAKU4AWrx1V38847QKJdryvLWNmOXy0ryf1LvX2HKnvcvgjrPKQ@mail.gmail.com>
	<CAMbWs48wLYmDZB47ZguQg6gncGkD_y6VWNtO_2=kppOb08dDOg@mail.gmail.com>

Hi Richard Guo
    Is it necessary to add some comments here?


+ if (!innerpath->parallel_safe ||
+ !bms_is_empty(PATH_REQ_OUTER(innerpath)))
+ continue;
+
+ if (matched_path != NULL &&
+ compare_path_costs(matched_path, innerpath, TOTAL_COST) <= 0)
+ continue;
+
+ matched_path = innerpath;

Richard Guo <[email protected]> 于2024年1月10日周三 15:08写道:

>
> On Tue, Apr 11, 2023 at 11:43 AM Andy Fan <[email protected]>
> wrote:
>
>> On Tue, Apr 11, 2023 at 11:03 AM Richard Guo <[email protected]>
>> 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 not sure about this conclusion.  Surely we can depend on that the
> partial_pathlist is kept sorted by total_cost ASC.  This is emphasized
> in the comment of add_partial_path, and also leveraged in practice, such
> as in many places we just use linitial(rel->partial_pathlist) as the
> cheapest partial path.
>
> But get_cheapest_parallel_safe_total_inner works on pathlist not
> partial_pathlist.  And for pathlist, I'm not sure if it's a good
> practice to depend on its ordering.  Because
>
> 1) the comment of add_path only mentions that add_path_precheck relies
> on this ordering, but it does not mention that other functions also do;
>
> 2) other than add_path_precheck, I haven't observed any other functions
> that rely on this ordering.  The only exception as far as I notice is
> get_cheapest_parallel_safe_total_inner.
>
> On the other hand, if we declare that we can rely on the pathlist being
> sorted in ascending order by total_cost, we should update the comment
> for add_path to highlight this aspect.  We should also include a comment
> for get_cheapest_parallel_safe_total_inner to clarify why an early exit
> is possible, similar to what we do for add_path_precheck.  Additionally,
> in several places, we can optimize our code by taking advantage of this
> fact and terminate the iteration through the pathlist early when looking
> for the cheapest path of a certain kind.
>
> Thanks
> Richard
>


view thread (3+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Can we rely on the ordering of paths in pathlist?
  In-Reply-To: <CAGjGUAKiE5kmtpp2A--9RPFnGtUtooFAQ0rxZ33QzA0Hn3pGqw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox