Re: [Patch] remove duplicated smgrclose

From: Cary Huang <cary(dot)huang(at)highgo(dot)ca>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Steven Niu <niushiji(at)gmail(dot)com>
Subject: Re: [Patch] remove duplicated smgrclose
Date: 2024-08-23 21:15:29
Message-ID: 172444772995.382822.12616136179121614869.pgcf@coridan.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation: not tested

Hi

the patch looks good to me as well. Calling smgrclose() right after calling smgrdounlinkall() does seem
unnecessary as it is already done inside smgrdounlinkall() as you mentioned. I checked the commit logs
and seems like the code has been like this for over 10 years. One difference is that smgrdounlinkall() does
not reset smgr_cached_nblocks and smgr_targblock but that does not seem to matter as it is about to
remove the physical files.

While leaving them like this does no harm because smgrclose() simply does nothing if the relation has already
been closed, it does look weird that the code tries to close the relation after smgrdounlinkall(), because the
physical files have just been deleted when smgrdounlinkall() completes, and we try to close something that
has been deleted ?!

---------------------------
Cary Huang
Highgo software Canada
www.highgo.ca

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Samuel Marks 2024-08-23 21:25:54 Re: [PATCH] Guard `CLOBBER_FREED_MEMORY` & `MEMORY_CONTEXT_CHECKING`
Previous Message Tom Lane 2024-08-23 21:11:30 Re: [PATCH] Guard `CLOBBER_FREED_MEMORY` & `MEMORY_CONTEXT_CHECKING`