Hi, On 2022-08-03 16:45:23 +0530, Dilip Kumar wrote: > Another version of the patch which closes the smgr at the end using > smgrcloserellocator() and I have also added a commit message.
What's the motivation behind the explicit close? > @@ -258,8 +258,8 @@ ScanSourceDatabasePgClass(Oid tbid, Oid dbid, char > *srcpath) > Page page; > List *rlocatorlist = NIL; > LockRelId relid; > - Relation rel; > Snapshot snapshot; > + SMgrRelation smgr; > BufferAccessStrategy bstrategy; > > /* Get pg_class relfilenumber. */ > @@ -276,16 +276,9 @@ ScanSourceDatabasePgClass(Oid tbid, Oid dbid, char > *srcpath) > rlocator.dbOid = dbid; > rlocator.relNumber = relfilenumber; > > - /* > - * We can't use a real relcache entry for a relation in some other > - * database, but since we're only going to access the fields related to > - * physical storage, a fake one is good enough. If we didn't do this and > - * used the smgr layer directly, we would have to worry about > - * invalidations. > - */ > - rel = CreateFakeRelcacheEntry(rlocator); > - nblocks = smgrnblocks(RelationGetSmgr(rel), MAIN_FORKNUM); > - FreeFakeRelcacheEntry(rel); > + smgr = smgropen(rlocator, InvalidBackendId); > + nblocks = smgrnblocks(smgr, MAIN_FORKNUM); > + smgrclose(smgr); Why are you opening and then closing again? Part of the motivation for the question is that a local SMgrRelation variable may lead to it being used further, opening up interrupt processing issues. > + rlocator.locator = src_rlocator; > + smgrcloserellocator(rlocator); > + > + rlocator.locator = dst_rlocator; > + smgrcloserellocator(rlocator); As mentioned above, it's not clear to me why this is now done... Otherwise looks good to me. Greetings, Andres Freund
