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


Reply via email to