Re: [Patch] remove duplicated smgrclose

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_9Jww@mail.gmail.com
[2] https://commitfest.postgresql.org/patch/5149/
[3] https://postgr.es/m/CAD21AoBLePJO5NtDoZWRxprCOtauMFr6Uaj4V8FxWJ1rKeyZFw@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)