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
