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
