RE: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

On Wed, Aug 18, 2021 9:17 AM houzj(dot)fnst(at)fujitsu(dot)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