public inbox for [email protected]
help / color / mirror / Atom feedFrom: Richard Guo <[email protected]>
To: David Rowley <[email protected]>
Cc: Julien Rouhaud <[email protected]>
Cc: 謝東霖 <[email protected]>
Cc: [email protected]
Subject: Re: Improve join_search_one_level readibilty (one line change)
Date: Fri, 4 Aug 2023 12:05:14 +0800
Message-ID: <CAMbWs4_QKD4tNn6mmY7-u-HrzHcN4PM8-ope8vYtmpDkOuGXGQ@mail.gmail.com> (raw)
In-Reply-To: <CAApHDvrDPvj51Te76fOV+Yu6MvYKzFVmx4hZ0wZ0TmDqZ+R3+Q@mail.gmail.com>
References: <CANWNU8x9P9aCXGF=aT-A_8mLTAT0LkcZ_ySYrGbcuHzMQw2-1g@mail.gmail.com>
<CANWNU8xm07jYUHxGh3XNHtcY37z+56-6bDD4piPt6=KidiHshQ@mail.gmail.com>
<CAOBaU_YTCo6F2kkhJy8jWmw--iDYHTEWeENunyu0Bc27VfO_KQ@mail.gmail.com>
<CANWNU8yxZ-SwGJvbbPuPmz2dUmn27q5CL885MWaD16+y-QuGEw@mail.gmail.com>
<20230731134837.dpugorvuhtv27b3t@jrouhaud>
<CAApHDvrDPvj51Te76fOV+Yu6MvYKzFVmx4hZ0wZ0TmDqZ+R3+Q@mail.gmail.com>
On Fri, Aug 4, 2023 at 10:36 AM David Rowley <[email protected]> wrote:
> The whole lnext() stuff all feels a bit old now that Lists are arrays.
> I think we'd be better adjusting the code to pass the List index where
> we start from rather than the ListCell to start from. That way we can
> use for_each_from() to iterate rather than for_each_cell(). What's
> there today feels a bit crufty and there's some element of danger that
> the given ListCell does not even belong to the given List.
I think we can go even further to do the same for 'bushy plans' case,
like the attached.
Thanks
Richard
Attachments:
[application/octet-stream] v2-0001-join_search_one_level-tidyup.patch (4.1K, ../CAMbWs4_QKD4tNn6mmY7-u-HrzHcN4PM8-ope8vYtmpDkOuGXGQ@mail.gmail.com/3-v2-0001-join_search_one_level-tidyup.patch)
download | inline diff:
From f240e35134aa1fa1f7e24e07ff6c8cdcdf85d81c Mon Sep 17 00:00:00 2001
From: pgsql-guo <[email protected]>
Date: Fri, 4 Aug 2023 11:25:23 +0800
Subject: [PATCH v2] join_search_one_level tidyup
---
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..5a1e8ba73d 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);
--
2.32.0
view thread (5+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Improve join_search_one_level readibilty (one line change)
In-Reply-To: <CAMbWs4_QKD4tNn6mmY7-u-HrzHcN4PM8-ope8vYtmpDkOuGXGQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox