Thread: Fix typo about generate_gather_paths

Hi

Since ba3e76c,
the optimizer call generate_useful_gather_paths instead of generate_gather_paths() outside.

But I noticed that some comment still talking about generate_gather_paths not generate_useful_gather_paths.
I think we should fix these comment, and I try to replace these " generate_gather_paths " with "
generate_useful_gather_paths" 

Best regards,
houzj

On 12/9/20 3:21 AM, Hou, Zhijie wrote:
> Hi
> 
> Since ba3e76c,
> the optimizer call generate_useful_gather_paths instead of generate_gather_paths() outside.
> 
> But I noticed that some comment still talking about generate_gather_paths not generate_useful_gather_paths.
> I think we should fix these comment, and I try to replace these " generate_gather_paths " with "
generate_useful_gather_paths"
 
> 

Thanks. I started looking at this a bit more closely, and I think most 
of the changes are fine - the code was changed to call a different 
function, but the comments still reference generate_gather_paths().

The one exception seems to be create_ordered_paths(), because that 
comment also makes statements about what generate_gather_pathes is 
doing. And some of it does not apply to generate_useful_gather_paths.
For example it says it generates order-preserving Gather Merge paths, 
but generate_useful_gather_paths also generates paths with sorts (which 
are clearly not order-preserving).

So I think this comment will need a bit more work to update ...


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi,

On Wed, Dec 23, 2020 at 3:24 AM Tomas Vondra
<tomas.vondra@enterprisedb.com> wrote:
>
> On 12/9/20 3:21 AM, Hou, Zhijie wrote:
> > Hi
> >
> > Since ba3e76c,
> > the optimizer call generate_useful_gather_paths instead of generate_gather_paths() outside.
> >
> > But I noticed that some comment still talking about generate_gather_paths not generate_useful_gather_paths.
> > I think we should fix these comment, and I try to replace these " generate_gather_paths " with "
generate_useful_gather_paths" 
> >
>
> Thanks. I started looking at this a bit more closely, and I think most
> of the changes are fine - the code was changed to call a different
> function, but the comments still reference generate_gather_paths().
>
> The one exception seems to be create_ordered_paths(), because that
> comment also makes statements about what generate_gather_pathes is
> doing. And some of it does not apply to generate_useful_gather_paths.
> For example it says it generates order-preserving Gather Merge paths,
> but generate_useful_gather_paths also generates paths with sorts (which
> are clearly not order-preserving).
>
> So I think this comment will need a bit more work to update ...

Status update for a commitfest entry.

This patch has been "Waiting on Author" without seeing any activity
since Tomas sent review comments. I'm planning to set it to "Returned
with Feedback”, barring objections.

Regards,

--
Masahiko Sawada
EDB:  https://www.enterprisedb.com/

Hi,

On Mon, Feb 1, 2021 at 11:44 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> Hi,
>
> On Wed, Dec 23, 2020 at 3:24 AM Tomas Vondra
> <tomas.vondra@enterprisedb.com> wrote:
> >
> > On 12/9/20 3:21 AM, Hou, Zhijie wrote:
> > > Hi
> > >
> > > Since ba3e76c,
> > > the optimizer call generate_useful_gather_paths instead of generate_gather_paths() outside.
> > >
> > > But I noticed that some comment still talking about generate_gather_paths not generate_useful_gather_paths.
> > > I think we should fix these comment, and I try to replace these " generate_gather_paths " with "
generate_useful_gather_paths" 
> > >
> >
> > Thanks. I started looking at this a bit more closely, and I think most
> > of the changes are fine - the code was changed to call a different
> > function, but the comments still reference generate_gather_paths().
> >
> > The one exception seems to be create_ordered_paths(), because that
> > comment also makes statements about what generate_gather_pathes is
> > doing. And some of it does not apply to generate_useful_gather_paths.
> > For example it says it generates order-preserving Gather Merge paths,
> > but generate_useful_gather_paths also generates paths with sorts (which
> > are clearly not order-preserving).
> >
> > So I think this comment will need a bit more work to update ...
>
> Status update for a commitfest entry.
>
> This patch has been "Waiting on Author" without seeing any activity
> since Tomas sent review comments. I'm planning to set it to "Returned
> with Feedback”, barring objections.
>

This patch, which you submitted to this CommitFest, has been awaiting
your attention for more than one month. As such, we have moved it to
"Returned with Feedback" and removed it from the reviewing queue.
Depending on timing, this may be reversable, so let us know if there
are extenuating circumstances. In any case, you are welcome to address
the feedback you have received, and resubmit the patch to the next
CommitFest.

Thank you for contributing to PostgreSQL.

Regards,

--
Masahiko Sawada
EDB:  https://www.enterprisedb.com/