Horton

Re: [Patch] remove duplicated smgrclose

Cary Huang <cary.huang@highgo.ca>

View thread
From: Cary Huang <cary.huang@highgo.ca>
To: pgsql-hackers@lists.postgresql.org
Cc: Steven Niu <niushiji@gmail.com>
Date: 2024-08-23T21:15:29Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Give SMgrRelation pointers a well-defined lifetime.

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