From: | Julien Rouhaud <rjuju123(at)gmail(dot)com> |
---|---|
To: | 謝東霖 <douenergy(at)gmail(dot)com> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Improve join_search_one_level readibilty (one line change) |
Date: | 2023-07-31 13:48:37 |
Message-ID: | 20230731134837.dpugorvuhtv27b3t@jrouhaud |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
From | Date | Subject | |
---|---|---|---|
Next Message | José Neves | 2023-07-31 14:16:22 | RE: CDC/ETL system on top of logical replication with pgoutput, custom client |
Previous Message | Amit Kapila | 2023-07-31 13:31:07 | Re: CDC/ETL system on top of logical replication with pgoutput, custom client |