Hi,
On 2026-04-08 13:37:31 -0500, Nathan Bossart wrote:
> On Wed, Apr 08, 2026 at 01:28:06PM -0500, Sami Imseih wrote:
> >> + if (AmAutoVacuumWorkerProcess())
> >> + pfree(tabentry);
> >> }
> >
> > This works too, but v1-0001 is more generalized and we don't have to
> > care about who the caller is when deciding to free or not.
>
> Well, I think if we were building this view from scratch without any
> knowledge of autovacuum, we probably wouldnt bother manually freeing the
> stats entries based on the value of the GUC. We'd probably just let the
> commit/abort code take care of it, which AFAICT is what we do elsewhere.
> Manually freeing the entries is also risky, which commit 02502c1bca touches
> on:
>
> Note: pfree'ing the PgStat_StatTabEntry structs here seems a bit
> risky, because pgstat_fetch_stat_tabentry_ext does not guarantee
> anything about whether its result is long-lived. It appears okay
> so long as autovacuum forces PGSTAT_FETCH_CONSISTENCY_NONE, but
> I think that API could use a re-think.
>
> This seems very much to be an autovacuum-specific hack that we shouldn't be
> propagating elsewhere.
Agreed. There aren't that many callers of pgstat_fetch_entry (even if you
count indirect ones), I think the best path might be to add a bool *may_free
argument where necessary and just deal with the size of change that brings
with it.
Greetings,
Andres Freund
