Tidy up join_search_one_level code

The code in join_search_one_level was a bit convoluted.  With a casual
glance, you might think that other_rels_list was being set to something
other than joinrels[1] when level == 2, however, joinrels[level - 1] is
joinrels[1] when level == 2, so nothing special needs to happen to set
other_rels_list.  Let's clean that up to avoid confusing anyone.

In passing, we may as well modernize the loop in
make_rels_by_clause_joins() and instead of passing in the ListCell to
start looping from, let's just pass in the index where to start from and
make use of for_each_from().  Ever since 1cff1b95a, Lists are arrays
under the hood. lnext() and list_head() both seem a little too linked-list
like.

Author: Alex Hsieh, David Rowley, Richard Guo
Reviewed-by: Julien Rouhaud
Discussion: https://postgr.es/m/CANWNU8x9P9aCXGF%3DaT-A_8mLTAT0LkcZ_ySYrGbcuHzMQw2-1g%40mail.gmail.com

diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c
index 015a0b3cbe033431fecdbf3f98f28b74eb2e4642..d03ace50a1b02b3cda7876919ac85a0e633a6922 100644 (file)
--- a/src/backend/optimizer/path/joinrels.c
+++ b/src/backend/optimizer/path/joinrels.c
@@ -25,8 +25,8 @@
 
 static void make_rels_by_clause_joins(PlannerInfo *root,
                                      RelOptInfo *old_rel,
-                                     List *other_rels_list,
-                                     ListCell *other_rels);
+                                     List *other_rels,
+                                     int first_rel_idx);
 static void make_rels_by_clauseless_joins(PlannerInfo *root,
                                          RelOptInfo *old_rel,
                                          List *other_rels);
@@ -93,6 +93,8 @@ join_search_one_level(PlannerInfo *root, int level)
        if (old_rel->joininfo != NIL || old_rel->has_eclass_joins ||
            has_join_restriction(root, old_rel))
        {
+           int         first_rel;
+
            /*
             * There are join clauses or join order restrictions relevant to
             * this rel, so consider joins between this rel and (only) those
@@ -106,24 +108,12 @@ join_search_one_level(PlannerInfo *root, int level)
             * to each initial rel they don't already include but have a join
             * clause or restriction with.
             */
-           List       *other_rels_list;
-           ListCell   *other_rels;
-
            if (level == 2)     /* consider remaining initial rels */
-           {
-               other_rels_list = joinrels[level - 1];
-               other_rels = lnext(other_rels_list, r);
-           }
-           else                /* consider all initial rels */
-           {
-               other_rels_list = joinrels[1];
-               other_rels = list_head(other_rels_list);
-           }
+               first_rel = foreach_current_index(r) + 1;
+           else
+               first_rel = 0;
 
-           make_rels_by_clause_joins(root,
-                                     old_rel,
-                                     other_rels_list,
-                                     other_rels);
+           make_rels_by_clause_joins(root, old_rel, joinrels[1], first_rel);
        }
        else
        {
@@ -167,8 +157,7 @@ join_search_one_level(PlannerInfo *root, int level)
        foreach(r, joinrels[k])
        {
            RelOptInfo *old_rel = (RelOptInfo *) lfirst(r);
-           List       *other_rels_list;
-           ListCell   *other_rels;
+           int         first_rel;
            ListCell   *r2;
 
            /*
@@ -180,19 +169,12 @@ join_search_one_level(PlannerInfo *root, int level)
                !has_join_restriction(root, old_rel))
                continue;
 
-           if (k == other_level)
-           {
-               /* only consider remaining rels */
-               other_rels_list = joinrels[k];
-               other_rels = lnext(other_rels_list, r);
-           }
+           if (k == other_level)   /* only consider remaining rels */
+               first_rel = foreach_current_index(r) + 1;
            else
-           {
-               other_rels_list = joinrels[other_level];
-               other_rels = list_head(other_rels_list);
-           }
+               first_rel = 0;
 
-           for_each_cell(r2, other_rels_list, other_rels)
+           for_each_from(r2, joinrels[other_level], first_rel)
            {
                RelOptInfo *new_rel = (RelOptInfo *) lfirst(r2);
 
@@ -286,9 +268,8 @@ join_search_one_level(PlannerInfo *root, int level)
  * automatically ensures that each new joinrel is only added to the list once.
  *
  * 'old_rel' is the relation entry for the relation to be joined
- * 'other_rels_list': a list containing the other
- * rels to be considered for joining
- * 'other_rels': the first cell to be considered
+ * 'other_rels': a list containing the other rels to be considered for joining
+ * 'first_rel_idx': the first rel to be considered in 'other_rels'
  *
  * Currently, this is only used with initial rels in other_rels, but it
  * will work for joining to joinrels too.
@@ -296,12 +277,12 @@ join_search_one_level(PlannerInfo *root, int level)
 static void
 make_rels_by_clause_joins(PlannerInfo *root,
                          RelOptInfo *old_rel,
-                         List *other_rels_list,
-                         ListCell *other_rels)
+                         List *other_rels,
+                         int first_rel_idx)
 {
    ListCell   *l;
 
-   for_each_cell(l, other_rels_list, other_rels)
+   for_each_from(l, other_rels, first_rel_idx)
    {
        RelOptInfo *other_rel = (RelOptInfo *) lfirst(l);