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...@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.
