At Tue, 14 Feb 2023 15:43:26 +0100, "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com> wrote in
> Oh right, my bad (the issue has been introduced in V2).
> Fixed in V4.
Great!
> > I thought that we might be able to return entry_ref->pending since the
> > callers don't call pfree on the returned pointer, but it is not great
> > that we don't inform the callers if the returned memory can be safely
> > pfreed or not.
> > Thus what I have in mind is the following.
> >
> >> if (!entry_ref)
> >> + {
> >> entry_ref = pgstat_fetch_pending_entry(PGSTAT_KIND_RELATION,
> >> InvalidOid, rel_id);
> >> + if (!entry_ref)
> >> + return NULL;
> >> + }
>
> LGTM, done that way in V4.
That part looks good to me, thanks!
I was going through v4 and it seems to me that the comment for
find_tabstat_entry may not be quite right.
> * find any existing PgStat_TableStatus entry for rel
> *
> * Find any existing PgStat_TableStatus entry for rel_id in the current
> * database. If not found, try finding from shared tables.
> *
> * If no entry found, return NULL, don't create a new one
The comment assumed that the function directly returns an entry from
shared memory, but now it copies the entry's contents into a palloc'ed
memory and stores the sums of some counters for the current
transaction in it. Do you think we should update the comment to
reflect this change?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center