Optimizer docs typos

Started by Daniel Gustafssonover 5 years ago10 messages

Daniel Gustafsson

daniel@yesql.se

over 5 years ago

1 attachment(s)

Attached diff fixes two small typos in the optimizer README.

cheers ./daniel

Attachments:

optimizer_doc_typos.diffapplication/octet-stream; name=optimizer_doc_typos.diff; x-unix-mode=0644Download

diff --git a/src/backend/optimizer/README b/src/backend/optimizer/README
index 40558cdf6b..7dcab9a54b 100644
--- a/src/backend/optimizer/README
+++ b/src/backend/optimizer/README
@@ -1109,10 +1109,10 @@ compatibly partitioned tables.
 Even if the joining relations don't have exactly the same partition bounds,
 partitionwise join can still be applied by using an advanced
 partition-matching algorithm.  For both the joining relations, the algorithm
-checks wether every partition of one joining relation only matches one
+checks whether every partition of one joining relation only matches one
 partition of the other joining relation at most.  In such a case the join
 between the joining relations can be broken down into joins between the
-matching partitions.  The join relation can then be considerd partitioned.
+matching partitions.  The join relation can then be considered partitioned.
 The algorithm produces the pairs of the matching partitions, plus the
 partition bounds for the join relation, to allow partitionwise join for
 computing the join.  The algorithm is implemented in partition_bounds_merge().

Magnus Hagander

magnus@hagander.net

over 5 years ago

In reply to: Daniel Gustafsson (#1)

Re: Optimizer docs typos

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/>

Etsuro Fujita

etsuro.fujita@gmail.com

over 5 years ago

In reply to: Magnus Hagander (#2)

Re: Optimizer docs typos

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

Richard Guo

guofenglinux@gmail.com

over 5 years ago

In reply to: Etsuro Fujita (#3)

Re: Optimizer docs typos

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:

Show quoted text

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

Etsuro Fujita

etsuro.fujita@gmail.com

over 5 years ago

In reply to: Richard Guo (#4)

Re: Optimizer docs typos

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

Etsuro Fujita

etsuro.fujita@gmail.com

over 5 years ago

In reply to: Etsuro Fujita (#5)

1 attachment(s)

Re: Optimizer docs typos

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

Attachments:

fix-indentation.patchapplication/octet-stream; name=fix-indentation.patchDownload

diff --git a/src/backend/optimizer/README b/src/backend/optimizer/README
index 7dcab9a54b..d174b8cb73 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
@@ -325,7 +325,7 @@ set up for recursive handling of subqueries
       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()
+------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()

Kyotaro Horiguchi

horikyota.ntt@gmail.com

over 5 years ago

In reply to: Etsuro Fujita (#6)

Re: Optimizer docs typos

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

Etsuro Fujita

etsuro.fujita@gmail.com

over 5 years ago

In reply to: Kyotaro Horiguchi (#7)

Re: Optimizer docs typos

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

Etsuro Fujita

etsuro.fujita@gmail.com

over 5 years ago

In reply to: Etsuro Fujita (#6)

Re: Optimizer docs typos

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

Richard Guo

guofenglinux@gmail.com

over 5 years ago

In reply to: Etsuro Fujita (#9)

Re: Optimizer docs typos

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

Done.

Thanks!

Thanks
Richard