Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 6c00d2c

Browse files
committed
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 1cff1b9, 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
1 parent 81ccbe5 commit 6c00d2c

File tree

1 file changed

+18
-37
lines changed

1 file changed

+18
-37
lines changed

src/backend/optimizer/path/joinrels.c

+18-37
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@
2525

2626
static void make_rels_by_clause_joins(PlannerInfo *root,
2727
RelOptInfo *old_rel,
28-
List *other_rels_list,
29-
ListCell *other_rels);
28+
List *other_rels,
29+
int first_rel_idx);
3030
static void make_rels_by_clauseless_joins(PlannerInfo *root,
3131
RelOptInfo *old_rel,
3232
List *other_rels);
@@ -93,6 +93,8 @@ join_search_one_level(PlannerInfo *root, int level)
9393
if (old_rel->joininfo != NIL || old_rel->has_eclass_joins ||
9494
has_join_restriction(root, old_rel))
9595
{
96+
int first_rel;
97+
9698
/*
9799
* There are join clauses or join order restrictions relevant to
98100
* this rel, so consider joins between this rel and (only) those
@@ -106,24 +108,12 @@ join_search_one_level(PlannerInfo *root, int level)
106108
* to each initial rel they don't already include but have a join
107109
* clause or restriction with.
108110
*/
109-
List *other_rels_list;
110-
ListCell *other_rels;
111-
112111
if (level == 2) /* consider remaining initial rels */
113-
{
114-
other_rels_list = joinrels[level - 1];
115-
other_rels = lnext(other_rels_list, r);
116-
}
117-
else /* consider all initial rels */
118-
{
119-
other_rels_list = joinrels[1];
120-
other_rels = list_head(other_rels_list);
121-
}
112+
first_rel = foreach_current_index(r) + 1;
113+
else
114+
first_rel = 0;
122115

123-
make_rels_by_clause_joins(root,
124-
old_rel,
125-
other_rels_list,
126-
other_rels);
116+
make_rels_by_clause_joins(root, old_rel, joinrels[1], first_rel);
127117
}
128118
else
129119
{
@@ -167,8 +157,7 @@ join_search_one_level(PlannerInfo *root, int level)
167157
foreach(r, joinrels[k])
168158
{
169159
RelOptInfo *old_rel = (RelOptInfo *) lfirst(r);
170-
List *other_rels_list;
171-
ListCell *other_rels;
160+
int first_rel;
172161
ListCell *r2;
173162

174163
/*
@@ -180,19 +169,12 @@ join_search_one_level(PlannerInfo *root, int level)
180169
!has_join_restriction(root, old_rel))
181170
continue;
182171

183-
if (k == other_level)
184-
{
185-
/* only consider remaining rels */
186-
other_rels_list = joinrels[k];
187-
other_rels = lnext(other_rels_list, r);
188-
}
172+
if (k == other_level) /* only consider remaining rels */
173+
first_rel = foreach_current_index(r) + 1;
189174
else
190-
{
191-
other_rels_list = joinrels[other_level];
192-
other_rels = list_head(other_rels_list);
193-
}
175+
first_rel = 0;
194176

195-
for_each_cell(r2, other_rels_list, other_rels)
177+
for_each_from(r2, joinrels[other_level], first_rel)
196178
{
197179
RelOptInfo *new_rel = (RelOptInfo *) lfirst(r2);
198180

@@ -286,22 +268,21 @@ join_search_one_level(PlannerInfo *root, int level)
286268
* automatically ensures that each new joinrel is only added to the list once.
287269
*
288270
* 'old_rel' is the relation entry for the relation to be joined
289-
* 'other_rels_list': a list containing the other
290-
* rels to be considered for joining
291-
* 'other_rels': the first cell to be considered
271+
* 'other_rels': a list containing the other rels to be considered for joining
272+
* 'first_rel_idx': the first rel to be considered in 'other_rels'
292273
*
293274
* Currently, this is only used with initial rels in other_rels, but it
294275
* will work for joining to joinrels too.
295276
*/
296277
static void
297278
make_rels_by_clause_joins(PlannerInfo *root,
298279
RelOptInfo *old_rel,
299-
List *other_rels_list,
300-
ListCell *other_rels)
280+
List *other_rels,
281+
int first_rel_idx)
301282
{
302283
ListCell *l;
303284

304-
for_each_cell(l, other_rels_list, other_rels)
285+
for_each_from(l, other_rels, first_rel_idx)
305286
{
306287
RelOptInfo *other_rel = (RelOptInfo *) lfirst(l);
307288

0 commit comments

Comments
 (0)