Horton

Thread

Commits

  1. Adjust indentation in src/backend/optimizer/README.

  1. Optimizer docs typos

    Daniel Gustafsson <daniel@yesql.se> — 2020-05-18T09:31:14Z

    Attached diff fixes two small typos in the optimizer README.

cheers ./daniel

  2. Re: Optimizer docs typos

    Magnus Hagander <magnus@hagander.net> — 2020-05-18T09:55:59Z

    On Mon, May 18, 2020 at 11:31 AM Daniel Gustafsson <daniel@yesql.se> wrote:

> Attached diff fixes two small typos in the optimizer README.
>

Pushed, thanks.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ <http://www.hagander.net/>
 Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

  3. Re: Optimizer docs typos

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2020-05-18T10:00:30Z

    On Mon, May 18, 2020 at 6:56 PM Magnus Hagander <magnus@hagander.net> wrote:
> On Mon, May 18, 2020 at 11:31 AM Daniel Gustafsson <daniel@yesql.se> wrote:
>> Attached diff fixes two small typos in the optimizer README.

> Pushed, thanks.

Thank you!

Best regards,
Etsuro Fujita

  4. Re: Optimizer docs typos

    Richard Guo <guofenglinux@gmail.com> — 2020-05-18T10:45:11Z

    In this same README doc, another suspicious typo to me, which happens in
section "Optimizer Functions", is in the prefix to query_planner(),
we should have three dashes, rather than two, since query_planner() is
called within grouping_planner().

diff --git a/src/backend/optimizer/README b/src/backend/optimizer/README
index 7dcab9a..bace081 100644
--- a/src/backend/optimizer/README
+++ b/src/backend/optimizer/README
@@ -315,7 +315,7 @@ set up for recursive handling of subqueries
   preprocess target list for non-SELECT queries
   handle UNION/INTERSECT/EXCEPT, GROUP BY, HAVING, aggregates,
        ORDER BY, DISTINCT, LIMIT
---query_planner()
+---query_planner()
    make list of base relations used in query
    split up the qual into restrictions (a=1) and joins (b=c)
    find qual clauses that enable merge and hash joins

Thanks
Richard

On Mon, May 18, 2020 at 6:00 PM Etsuro Fujita <etsuro.fujita@gmail.com>
wrote:

> On Mon, May 18, 2020 at 6:56 PM Magnus Hagander <magnus@hagander.net>
> wrote:
> > On Mon, May 18, 2020 at 11:31 AM Daniel Gustafsson <daniel@yesql.se>
> wrote:
> >> Attached diff fixes two small typos in the optimizer README.
>
> > Pushed, thanks.
>
> Thank you!
>
> Best regards,
> Etsuro Fujita
>
>
>

  5. Re: Optimizer docs typos

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2020-05-19T10:35:56Z

    On Mon, May 18, 2020 at 7:45 PM Richard Guo <guofenglinux@gmail.com> wrote:
> In this same README doc, another suspicious typo to me, which happens in
> section "Optimizer Functions", is in the prefix to query_planner(),
> we should have three dashes, rather than two, since query_planner() is
> called within grouping_planner().
>
> diff --git a/src/backend/optimizer/README b/src/backend/optimizer/README
> index 7dcab9a..bace081 100644
> --- a/src/backend/optimizer/README
> +++ b/src/backend/optimizer/README
> @@ -315,7 +315,7 @@ set up for recursive handling of subqueries
>    preprocess target list for non-SELECT queries
>    handle UNION/INTERSECT/EXCEPT, GROUP BY, HAVING, aggregates,
>         ORDER BY, DISTINCT, LIMIT
> ---query_planner()
> +---query_planner()
>     make list of base relations used in query
>     split up the qual into restrictions (a=1) and joins (b=c)
>     find qual clauses that enable merge and hash joins

Yeah, you are right.  Another one would be in the prefix to
standard_join_search(); I think it might be better to have six dashes,
rather than five, because standard_join_search() is called within
make_rel_from_joinlist().

Best regards,
Etsuro Fujita

  6. Re: Optimizer docs typos

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2020-05-20T10:17:48Z

    On Tue, May 19, 2020 at 7:35 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> On Mon, May 18, 2020 at 7:45 PM Richard Guo <guofenglinux@gmail.com> wrote:
> > In this same README doc, another suspicious typo to me, which happens in
> > section "Optimizer Functions", is in the prefix to query_planner(),
> > we should have three dashes, rather than two, since query_planner() is
> > called within grouping_planner().
> >
> > diff --git a/src/backend/optimizer/README b/src/backend/optimizer/README
> > index 7dcab9a..bace081 100644
> > --- a/src/backend/optimizer/README
> > +++ b/src/backend/optimizer/README
> > @@ -315,7 +315,7 @@ set up for recursive handling of subqueries
> >    preprocess target list for non-SELECT queries
> >    handle UNION/INTERSECT/EXCEPT, GROUP BY, HAVING, aggregates,
> >         ORDER BY, DISTINCT, LIMIT
> > ---query_planner()
> > +---query_planner()
> >     make list of base relations used in query
> >     split up the qual into restrictions (a=1) and joins (b=c)
> >     find qual clauses that enable merge and hash joins
>
> Yeah, you are right.  Another one would be in the prefix to
> standard_join_search(); I think it might be better to have six dashes,
> rather than five, because standard_join_search() is called within
> make_rel_from_joinlist().

Here is a patch including the change I proposed.  (Yet another thing I
noticed is the indent spaces for join_search_one_level(): that
function is called within standard_join_search(), so it would be
better to have one extra space, for consistency with others (eg,
set_base_rel_pathlists() called from make_one_rel()), but that would
be too nitpicking.)  This is more like an improvement, so I'll apply
the patch to HEAD only, if no objestions.

Best regards,
Etsuro Fujita

  7. Re: Optimizer docs typos

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2020-05-21T02:25:35Z

    At Wed, 20 May 2020 19:17:48 +0900, Etsuro Fujita <etsuro.fujita@gmail.com> wrote in 
> On Tue, May 19, 2020 at 7:35 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > On Mon, May 18, 2020 at 7:45 PM Richard Guo <guofenglinux@gmail.com> wrote:
> > > ---query_planner()
> > > +---query_planner()
> > >     make list of base relations used in query
> > >     split up the qual into restrictions (a=1) and joins (b=c)
> > >     find qual clauses that enable merge and hash joins
> >
> > Yeah, you are right.  Another one would be in the prefix to
> > standard_join_search(); I think it might be better to have six dashes,
> > rather than five, because standard_join_search() is called within
> > make_rel_from_joinlist().
> 
> Here is a patch including the change I proposed.  (Yet another thing I
> noticed is the indent spaces for join_search_one_level(): that
> function is called within standard_join_search(), so it would be
> better to have one extra space, for consistency with others (eg,
> set_base_rel_pathlists() called from make_one_rel()), but that would
> be too nitpicking.)  This is more like an improvement, so I'll apply
> the patch to HEAD only, if no objestions.

The original proposal for query_planner looks fine.

The description for make_rel_from_joinlist() and that for
standard_join_search() are at the same indentation depth.  And it is
also strange that seemingly there is no line for level-5
indentation. If we make standard_join_search() a 6th-hyphened level
item, indentation of the surrounding descriptions needs a fix.

----make_one_rel()
     set_base_rel_pathlists()
      find seqscan and all index paths for each base relation
      find selectivity of columns used in joins
     make_rel_from_joinlist()
      hand off join subproblems to a plugin, GEQO, or standard_join_search()
------standard_join_search()
      call join_search_one_level() for each level of join tree needed
      join_search_one_level():
        For each joinrel of the prior level, do make_rels_by_clause_joins()
        if it has join clauses, or make_rels_by_clauseless_joins() if not.

Looking the description for make_rel_from_joinlist(), it seems to me
that the author is thinking that make_rel_from_joinlist() is mere the
distributor for join subproblems and isn't worth an indent level.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  8. Re: Optimizer docs typos

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2020-05-21T06:36:10Z

    On Thu, May 21, 2020 at 11:25 AM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
> At Wed, 20 May 2020 19:17:48 +0900, Etsuro Fujita <etsuro.fujita@gmail.com> wrote in
> > Here is a patch including the change I proposed.  (Yet another thing I
> > noticed is the indent spaces for join_search_one_level(): that
> > function is called within standard_join_search(), so it would be
> > better to have one extra space, for consistency with others (eg,
> > set_base_rel_pathlists() called from make_one_rel()), but that would
> > be too nitpicking.)  This is more like an improvement, so I'll apply
> > the patch to HEAD only, if no objestions.

> The description for make_rel_from_joinlist() and that for
> standard_join_search() are at the same indentation depth.  And it is
> also strange that seemingly there is no line for level-5
> indentation. If we make standard_join_search() a 6th-hyphened level
> item, indentation of the surrounding descriptions needs a fix.
>
> ----make_one_rel()
>      set_base_rel_pathlists()
>       find seqscan and all index paths for each base relation
>       find selectivity of columns used in joins
>      make_rel_from_joinlist()
>       hand off join subproblems to a plugin, GEQO, or standard_join_search()
> ------standard_join_search()
>       call join_search_one_level() for each level of join tree needed
>       join_search_one_level():
>         For each joinrel of the prior level, do make_rels_by_clause_joins()
>         if it has join clauses, or make_rels_by_clauseless_joins() if not.

I don't think it's odd that we won't have 5-dashes indentation,
because I think we have 5-spaces indentation for
set_base_rel_pathlists() and make_rel_from_joinlist().  (I think the
dash indentation of optimizer functions such as standard_join_search()
just means that the dash-indented functions are more important
compared to other space-indented functions IMO.)  My point is that we
should adjust the dash or space indentation so that a deeper level of
indentation indicates that the outer optimizer function calls the
inner optimizer function.

Thanks!

Best regards,
Etsuro Fujita

  9. Re: Optimizer docs typos

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2020-05-22T06:53:21Z

    On Wed, May 20, 2020 at 7:17 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> On Tue, May 19, 2020 at 7:35 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > On Mon, May 18, 2020 at 7:45 PM Richard Guo <guofenglinux@gmail.com> wrote:
> > > In this same README doc, another suspicious typo to me, which happens in
> > > section "Optimizer Functions", is in the prefix to query_planner(),
> > > we should have three dashes, rather than two, since query_planner() is
> > > called within grouping_planner().
> > >
> > > diff --git a/src/backend/optimizer/README b/src/backend/optimizer/README
> > > index 7dcab9a..bace081 100644
> > > --- a/src/backend/optimizer/README
> > > +++ b/src/backend/optimizer/README
> > > @@ -315,7 +315,7 @@ set up for recursive handling of subqueries
> > >    preprocess target list for non-SELECT queries
> > >    handle UNION/INTERSECT/EXCEPT, GROUP BY, HAVING, aggregates,
> > >         ORDER BY, DISTINCT, LIMIT
> > > ---query_planner()
> > > +---query_planner()
> > >     make list of base relations used in query
> > >     split up the qual into restrictions (a=1) and joins (b=c)
> > >     find qual clauses that enable merge and hash joins
> >
> > Yeah, you are right.  Another one would be in the prefix to
> > standard_join_search(); I think it might be better to have six dashes,
> > rather than five, because standard_join_search() is called within
> > make_rel_from_joinlist().
>
> Here is a patch including the change I proposed.  (Yet another thing I
> noticed is the indent spaces for join_search_one_level(): that
> function is called within standard_join_search(), so it would be
> better to have one extra space, for consistency with others (eg,
> set_base_rel_pathlists() called from make_one_rel()), but that would
> be too nitpicking.)  This is more like an improvement, so I'll apply
> the patch to HEAD only, if no objestions.

Done.

Best regards,
Etsuro Fujita

  10. Re: Optimizer docs typos

    Richard Guo <guofenglinux@gmail.com> — 2020-05-22T08:29:42Z

    On Fri, May 22, 2020 at 2:53 PM Etsuro Fujita <etsuro.fujita@gmail.com>
wrote:

>
> Done.
>

Thanks!

Thanks
Richard