On Wed, Aug 18, 2021 at 8:23 AM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
>
> On Wed, Aug 18, 2021 9:17 AM houzj.fnst@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().
>
Don't we need to use BufFileDeleteShared instead of
SharedFileSetDelete as you have used to remove the changes file?
--
With Regards,
Amit Kapila.
