Hello, On 2024-Aug-09, Kirill Reshke wrote:
> Hi all! > This change looks good to me. However, i have an objection to these > lines from v2: > > > /* Close the forks at smgr level */ > > - for (forknum = 0; forknum <= MAX_FORKNUM; forknum++) > > - smgrsw[which].smgr_close(rels[i], forknum); > > + smgrclose(rels[i]); > > Why do we do this? I want to say that this does not appear to be to be an objection. I think it was just a question. An objection implies that you consider a change to be incorrect or detrimental, and therefore it should not be made. In this case, by posing your question in this way: > This seems to be an unrelated change given thread > $subj. This is just a pure refactoring job, which deserves a separate > patch. There is similar coding in > smgrdestroy function: [...] > So, I feel like these two places should be either changed together or > not be altered at all. And is it definitely a separate change. ... the patch was derailed and we ended up not doing anything at all, and worse, we ended up with a separate thread doing something closely related but in a way that appears unconnected to this thread. (Worse, Steven forgot to open a commitfest entry for it, so the patch was lost in the perpetual pgsql-hackers background noise). I think Masahiko rejected this patch because it was incorrect without that other patch, or something like that. I've marked this[2] entry as returned with feedback now. I would suggest to Steven to propose this patch again, but this time as a single patch that does the complete change rather than half here and half there, keeping Masahiko's closing comments[3] in mind. Once he has such a patch, he can reopen this commitfest entry as Needs Review (moving it to whatever the open commitfest is.) Thanks [1] https://postgr.es/m/CABBtG=d1Kkmi67VdM=jgaykq0+wgbhzpxwa3ms0s1db_j_9...@mail.gmail.com [2] https://commitfest.postgresql.org/patch/5149/ [3] https://postgr.es/m/cad21aoblepjo5ntdozwrxprcotaumfr6uaj4v8fxwj1rkey...@mail.gmail.com -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ Thou shalt study thy libraries and strive not to reinvent them without cause, that thy code may be short and readable and thy days pleasant and productive. (7th Commandment for C Programmers)
