public inbox for [email protected]  

help / color / mirror / Atom feed

From: Julien Rouhaud <[email protected]>
To: 謝東霖 <[email protected]>
Cc: [email protected]
Subject: Re: Improve join_search_one_level readibilty (one line change)
Date: Mon, 31 Jul 2023 21:48:37 +0800
Message-ID: <20230731134837.dpugorvuhtv27b3t@jrouhaud> (raw)
In-Reply-To: <CANWNU8yxZ-SwGJvbbPuPmz2dUmn27q5CL885MWaD16+y-QuGEw@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>

Hi,

On Wed, Jun 07, 2023 at 11:02:09AM +0800, 謝東霖 wrote:
> Thank you, Julien, for letting me know that cfbot doesn't test txt files.
> Much appreciated!

Thanks for posting this v2!

So unsurprisingly the cfbot is happy with this patch, since it doesn't change
the behavior at all.  I just have some nitpicking:

@@ -109,14 +109,14 @@ join_search_one_level(PlannerInfo *root, int level)
 			List	   *other_rels_list;
 			ListCell   *other_rels;

+			other_rels_list = joinrels[1];
+
 			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);
 			}


Since each branch only has a single instruction after the change the curly
braces aren't needed anymore.  The only reason keep them is if it helps
readability (like if there's a big comment associated), but that's not the case
here so it would be better to get rid of them.

Apart from that +1 from me for the patch, I think it helps focusing the
attention on what actually matters here.

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]
  Subject: Re: Improve join_search_one_level readibilty (one line change)
  In-Reply-To: <20230731134837.dpugorvuhtv27b3t@jrouhaud>

* 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