public inbox for [email protected]  

help / color / mirror / Atom feed

From: Lukas Fittl <[email protected]>
To: Haibo Yan <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: pg_buffercache: Add per-relation summary stats
Date: Tue, 24 Mar 2026 23:52:13 -0700
Message-ID: <CAP53Pkx3TkVWnMMiST3CVhu0uPqo9tdVn0s=_Our=EHtgs39nQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAP53Pkx0=ph0vG_M20yVAoK11yGSTZP=53-rZt36OCP4hBPaDQ@mail.gmail.com>
	<[email protected]>

Hi Haibo,

Thanks for your review!

On Mon, Mar 16, 2026 at 9:21 PM Haibo Yan <[email protected]> wrote:
> Could this use RelFileLocator plus ForkNumber instead of open-coding BufferRelStatsKey? That seems closer to existing PostgreSQL abstractions for physical relation identity.

Yes, that was noted by other reviewers as well, and makes sense.

> I wonder whether pg_buffercache_relation_stats() is the best name here. The function is really aggregating by relation file identity plus fork, and it is producing a summary of the current buffer contents rather than what many readers might assume from “relation stats”. Would something with summary be clearer than stats?

Per the most recent feedback, I'll rename this to
"pg_buffercache_relations" for now.

> Why are OUT relforknumber and OUT relfilenode exposed as int2 and oid respectively? Internally these are represented as ForkNumber and RelFileNumber, so I wonder whether the SQL interface should reflect that more clearly, or at least whether the current choice should be explained.

This is consistent with how pg_buffercache_pages represents them - I
think those are the correct mappings of the int ForkNumber (which we
know to be small in practice) and RelFileNumber is a typedef of Oid.

> The comment says, “Hash key for pg_buffercache_relation_stats — groups by relation identity”, but that seems imprecise. It is really grouping by relfilenode plus fork, i.e. physical relation-file identity rather than relation identity in a more logical sense.

Good point. I'll adapt this to "groups by relation file" for now.

> Is PARALLEL SAFE actually desirable here, as opposed to merely technically safe? A parallel query could cause multiple workers to perform full shared-buffer scans independently, which does not seem obviously desirable for this kind of diagnostic function.

I see your point, but I don't think a parallel plan would happen in
practice when just the function is being queried. Since other
pg_buffercache functions are also PARALLEL SAFE, I'll keep this as is
for now - if we want to adjust it we should be consistent I think.

Thanks,
Lukas

-- 
Lukas Fittl

view thread (27+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: pg_buffercache: Add per-relation summary stats
  In-Reply-To: <CAP53Pkx3TkVWnMMiST3CVhu0uPqo9tdVn0s=_Our=EHtgs39nQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox