On Wed, Aug 18, 2021 at 9:30 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> 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?
Yeah, it should be BufFileDeleteShared.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com