RE: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o - Mailing list pgsql-hackers

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().
Was it intentional ?

Best regards,
Houzj