On Wed, Aug 18, 2021 9:17 AM houzj.f...@fujitsu.com wrote: > Hi, > > I took a quick look at the v2 patch and noticed a typo. > > + * backends and render it read-only. If missing_ok is true then it will > return > + * NULL if file doesn not exist otherwise error. > */ > doesn not=> doesn't >
Here are some other comments: 1). +BufFileOpenShared(SharedFileSet *fileset, const char *name, int mode, + bool missing_ok) { BufFile *file; char segment_name[MAXPGPATH]; ... files = palloc(sizeof(File) * capacity); ... @@ -318,10 +320,15 @@ BufFileOpenShared(SharedFileSet *fileset, const char *name, int mode) * name. */ if (nfiles == 0) + { + if (missing_ok) + return NULL; + I think it might be better to pfree(files) when return NULL. 2). /* Delete the subxact file and release the memory, if it exist */ - if (ent->subxact_fileset) - { - subxact_filename(path, subid, xid); - SharedFileSetDeleteAll(ent->subxact_fileset); - pfree(ent->subxact_fileset); - ent->subxact_fileset = NULL; - } - - /* Remove the xid entry from the stream xid hash */ - hash_search(xidhash, (void *) &xid, HASH_REMOVE, NULL); + subxact_filename(path, subid, xid); + SharedFileSetDelete(xidfileset, path, true); Without the patch it doesn't throw an error if not exist, But with the patch, it pass error_on_failure=true to SharedFileSetDelete(). Was it intentional ? Best regards, Houzj