Can we remove the other_rels_list parameter for make_rels_by_clause_joins

Started by Andy Fanover 5 years ago1 messages
#1Andy Fan
zhihui.fan1213@gmail.com
1 attachment(s)

Hi:
For a given level for join_search_one_level, it is always try to join
every relation
in joinrel[level-1] to *initial_rels*. but the current code doesn't show
this directly.

join_search_one_level

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);
}

make_rels_by_clause_joins(root,

old_rel,

other_rels_list,

other_rels);

I'd like to remove the parameter and use root->inital_rels directly. I did
the same
for make_rels_by_clauseless_joins. The attached is my proposal which
should be
semantic correctly and more explicitly.

Best Regards
Andy Fan

Attachments:

v1-0001-Remove-other_rels_list-from-make_rels_by_clause_j.patchapplication/octet-stream; name=v1-0001-Remove-other_rels_list-from-make_rels_by_clause_j.patchDownload
From 6b7390b94e40056851f8a6a041f149cbe409e68b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E4=B8=80=E6=8C=83?= <yizhi.fzh@alibaba-inc.com>
Date: Thu, 30 Apr 2020 11:58:51 +0800
Subject: [PATCH v1] Remove other_rels_list from make_rels_by_clause_joins for
 better semantic clearly

---
 src/backend/optimizer/path/joinrels.c | 34 ++++++++-------------------
 1 file changed, 10 insertions(+), 24 deletions(-)

diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c
index 4e1650994d..5890a2d533 100644
--- a/src/backend/optimizer/path/joinrels.c
+++ b/src/backend/optimizer/path/joinrels.c
@@ -25,11 +25,9 @@
 
 static void make_rels_by_clause_joins(PlannerInfo *root,
 									  RelOptInfo *old_rel,
-									  List *other_rels_list,
 									  ListCell *other_rels);
 static void make_rels_by_clauseless_joins(PlannerInfo *root,
-										  RelOptInfo *old_rel,
-										  List *other_rels);
+										  RelOptInfo *old_rel);
 static bool has_join_restriction(PlannerInfo *root, RelOptInfo *rel);
 static bool has_legal_joinclause(PlannerInfo *root, RelOptInfo *rel);
 static bool restriction_is_constant_false(List *restrictlist,
@@ -106,23 +104,15 @@ 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);
-			}
+				other_rels = lnext(root->initial_rels, r);
 			else				/* consider all initial rels */
-			{
-				other_rels_list = joinrels[1];
-				other_rels = list_head(other_rels_list);
-			}
+				other_rels = list_head(root->initial_rels);
 
 			make_rels_by_clause_joins(root,
 									  old_rel,
-									  other_rels_list,
 									  other_rels);
 		}
 		else
@@ -140,8 +130,8 @@ join_search_one_level(PlannerInfo *root, int level)
 			 * avoid the duplicated effort.
 			 */
 			make_rels_by_clauseless_joins(root,
-										  old_rel,
-										  joinrels[1]);
+										  old_rel);
+
 		}
 	}
 
@@ -243,8 +233,8 @@ join_search_one_level(PlannerInfo *root, int level)
 			RelOptInfo *old_rel = (RelOptInfo *) lfirst(r);
 
 			make_rels_by_clauseless_joins(root,
-										  old_rel,
-										  joinrels[1]);
+										  old_rel);
+
 		}
 
 		/*----------
@@ -286,8 +276,6 @@ 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
  *
  * Currently, this is only used with initial rels in other_rels, but it
@@ -296,12 +284,11 @@ 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)
 {
 	ListCell   *l;
 
-	for_each_cell(l, other_rels_list, other_rels)
+	for_each_cell(l, root->initial_rels, other_rels)
 	{
 		RelOptInfo *other_rel = (RelOptInfo *) lfirst(l);
 
@@ -329,12 +316,11 @@ make_rels_by_clause_joins(PlannerInfo *root,
  */
 static void
 make_rels_by_clauseless_joins(PlannerInfo *root,
-							  RelOptInfo *old_rel,
-							  List *other_rels)
+							  RelOptInfo *old_rel)
 {
 	ListCell   *l;
 
-	foreach(l, other_rels)
+	foreach(l, root->initial_rels)
 	{
 		RelOptInfo *other_rel = (RelOptInfo *) lfirst(l);
 
-- 
2.21.0