From 6c00d2c9d4c533dad4e0afe28cfde0e12e4eb4e0 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Sun, 6 Aug 2023 21:51:54 +1200 Subject: [PATCH] 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 --- src/backend/optimizer/path/joinrels.c | 55 +++++++++------------------ 1 file changed, 18 insertions(+), 37 deletions(-) diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c index 015a0b3cbe..d03ace50a1 100644 --- 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);