Re: Improve join_search_one_level readibilty (one line change)

Hi,

On Sat, Jun 03, 2023 at 05:24:43PM +0800, 謝東霖 wrote:
>
> Attached is my first patch for PostgreSQL, which is a simple one-liner
> that I believe can improve the code.

Welcome!

> In the "join_search_one_level" function, I noticed that the variable
> "other_rels_list" always refers to "joinrels[1]" even when the (level
> == 2) condition is met. I propose changing:
>
> "other_rels_list = joinrels[level - 1]" to "other_rels_list = joinrels[1]"
>
> This modification aims to enhance clarity and avoid unnecessary instructions.

Agreed. It looks like it was originally introduced as mechanical changes in a
bigger patch. It would probably be better to move the other_rels_list
initialization out of the if instruction and put it with the variable
declaration, to make it even clearer. I'm not that familiar with this area of
the code so hopefully someone else will comment.

> I would greatly appreciate any review and feedback on this patch as I
> am a newcomer to PostgreSQL contributions. Your input will help me
> improve and contribute effectively to the project.

I think you did everything as needed! You should consider adding you patch to
the next opened commitfest (https://commitfest.postgresql.org/43/) if you
haven't already, to make sure it won't be forgotten, even if it's a one-liner.
It will then also be regularly tested by the cfbot (http://cfbot.cputube.org/).

If needed, you can also test the same CI jobs (covering multiple OS) using your
personal github account, see
https://github.com/postgres/postgres/blob/master/src/tools/ci/README on details
to set it up.

Best practice is also to review a patch of similar difficulty when sending one.
You can look at the same commit fest entry if anything interests you, and
register as a reviewer.

> Additionally, if anyone has any tips on ensuring that Gmail recognizes
> my attached patches as the "text/x-patch" MIME type when sending them
> from the Chrome client, I would be grateful for the advice.

I don't see any problem with the attachment. You can always check looking at
the online archive for that, for instance for your email:
https://www.postgresql.org/message-id/CANWNU8x9P9aCXGF=aT-A_8mLTAT0LkcZ_ySYrGbcuHzMQw2-1g@mail.gmail.com

As far as I know only apple mail is problematic with that regards, as it
doesn't send attachments as attachments.

> Or maybe the best practice is to use Git send-email ?

I don't think anyone uses git send-email on this mailing list. We usually
prefer to manually attach patch(es), possibly compressing them if they're big,
and then keep all the discussion and new revisions on the same thread.