Horton

Thread

Commits

  1. Fix confusion in comments about generate_gather_paths

  1. Fix typo about generate_gather_paths

    Hou, Zhijie <houzj.fnst@cn.fujitsu.com> — 2020-12-09T02:21:25Z

    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

  2. Re: Fix typo about generate_gather_paths

    Tomas Vondra <tomas.vondra@enterprisedb.com> — 2020-12-22T18:24:31Z

    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

  3. Re: Fix typo about generate_gather_paths

    Masahiko Sawada <sawada.mshk@gmail.com> — 2021-02-01T02:44:33Z

    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/

  4. Re: Fix typo about generate_gather_paths

    Masahiko Sawada <sawada.mshk@gmail.com> — 2021-02-01T13:23:16Z

    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/

  5. Re: Fix typo about generate_gather_paths

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2021-02-23T23:12:14Z

    On 2020-Dec-22, Tomas Vondra wrote:

> 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().

Hi, this was forgotten.  It seemed better to fix at least some of the
wrong references than not do anything, so I pushed the parts that seemed
100% correct.  Regarding this one:

> 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).

I left this one out.  If Hou or Tomas want to propose/push a further
patch, that'd be great.

Thanks!

-- 
Álvaro Herrera                            39°49'30"S 73°17'W
"I'm always right, but sometimes I'm more right than other times."
                                                  (Linus Torvalds)