From 40597bd30c6c9c4ef9a87864d4f6e2b2b0d7548f Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Wed, 12 Oct 2022 12:20:43 -0700
Subject: [PATCH v15 2/2] fixup! pg_buffercache: Add pg_buffercache_summary()

---
 .../expected/pg_buffercache.out               | 24 ++++++++
 .../pg_buffercache--1.3--1.4.sql              | 10 +++-
 contrib/pg_buffercache/pg_buffercache_pages.c | 27 ++-------
 contrib/pg_buffercache/sql/pg_buffercache.sql | 13 +++++
 doc/src/sgml/pgbuffercache.sgml               | 55 +++++++++++--------
 5 files changed, 80 insertions(+), 49 deletions(-)

diff --git a/contrib/pg_buffercache/expected/pg_buffercache.out b/contrib/pg_buffercache/expected/pg_buffercache.out
index 6798eb74322..635f01e3b21 100644
--- a/contrib/pg_buffercache/expected/pg_buffercache.out
+++ b/contrib/pg_buffercache/expected/pg_buffercache.out
@@ -17,3 +17,27 @@ from pg_buffercache_summary();
  t        | t        | t
 (1 row)
 
+-- Check that the functions / views can't be accessed by default. To avoid
+-- having to create a dedicated user, use the pg_database_owner pseudo-role.
+SET ROLE pg_database_owner;
+SELECT * FROM pg_buffercache;
+ERROR:  permission denied for view pg_buffercache
+SELECT * FROM pg_buffercache_pages() AS p (wrong int);
+ERROR:  permission denied for function pg_buffercache_pages
+SELECT * FROM pg_buffercache_summary();
+ERROR:  permission denied for function pg_buffercache_summary
+RESET role;
+-- Check that pg_monitor is allowed to query view / function
+SET ROLE pg_monitor;
+SELECT count(*) > 0 FROM pg_buffercache;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT buffers_used + buffers_unused > 0 FROM pg_buffercache_summary();
+ ?column? 
+----------
+ t
+(1 row)
+
diff --git a/contrib/pg_buffercache/pg_buffercache--1.3--1.4.sql b/contrib/pg_buffercache/pg_buffercache--1.3--1.4.sql
index 77e250b430f..8f212dc5e93 100644
--- a/contrib/pg_buffercache/pg_buffercache--1.3--1.4.sql
+++ b/contrib/pg_buffercache/pg_buffercache--1.3--1.4.sql
@@ -3,11 +3,15 @@
 -- complain if script is sourced in psql, rather than via ALTER EXTENSION
 \echo Use "ALTER EXTENSION pg_buffercache UPDATE TO '1.4'" to load this file. \quit
 
-CREATE FUNCTION pg_buffercache_summary()
-RETURNS TABLE (buffers_used int4, buffers_unused int4, buffers_dirty int4,
-				buffers_pinned int4, usagecount_avg real)
+CREATE FUNCTION pg_buffercache_summary(
+    OUT buffers_used int4,
+    OUT buffers_unused int4,
+    OUT buffers_dirty int4,
+    OUT buffers_pinned int4,
+    OUT usagecount_avg float8)
 AS 'MODULE_PATHNAME', 'pg_buffercache_summary'
 LANGUAGE C PARALLEL SAFE;
 
 -- Don't want these to be available to public.
 REVOKE ALL ON FUNCTION pg_buffercache_summary() FROM PUBLIC;
+GRANT EXECUTE ON FUNCTION pg_buffercache_summary() TO pg_monitor;
diff --git a/contrib/pg_buffercache/pg_buffercache_pages.c b/contrib/pg_buffercache/pg_buffercache_pages.c
index a8f9750ac42..1c6a2f22caa 100644
--- a/contrib/pg_buffercache/pg_buffercache_pages.c
+++ b/contrib/pg_buffercache/pg_buffercache_pages.c
@@ -253,22 +253,10 @@ pg_buffercache_summary(PG_FUNCTION_ARGS)
 	int32		buffers_unused = 0;
 	int32		buffers_dirty = 0;
 	int32		buffers_pinned = 0;
-	float		usagecount_avg = 0;
+	int64		usagecount_total = 0;
 
-	/* Construct a tuple descriptor for the result rows. */
-	tupledesc = CreateTemplateTupleDesc(NUM_BUFFERCACHE_SUMMARY_ELEM);
-	TupleDescInitEntry(tupledesc, (AttrNumber) 1, "buffers_used",
-					   INT4OID, -1, 0);
-	TupleDescInitEntry(tupledesc, (AttrNumber) 2, "buffers_unused",
-					   INT4OID, -1, 0);
-	TupleDescInitEntry(tupledesc, (AttrNumber) 3, "buffers_dirty",
-					   INT4OID, -1, 0);
-	TupleDescInitEntry(tupledesc, (AttrNumber) 4, "buffers_pinned",
-					   INT4OID, -1, 0);
-	TupleDescInitEntry(tupledesc, (AttrNumber) 5, "usagecount_avg",
-					   FLOAT4OID, -1, 0);
-
-	BlessTupleDesc(tupledesc);
+	if (get_call_result_type(fcinfo, NULL, &tupledesc) != TYPEFUNC_COMPOSITE)
+		elog(ERROR, "return type must be a row type");
 
 	for (int i = 0; i < NBuffers; i++)
 	{
@@ -287,7 +275,7 @@ pg_buffercache_summary(PG_FUNCTION_ARGS)
 		if (buf_state & BM_VALID)
 		{
 			buffers_used++;
-			usagecount_avg += BUF_STATE_GET_USAGECOUNT(buf_state);
+			usagecount_total += BUF_STATE_GET_USAGECOUNT(buf_state);
 
 			if (buf_state & BM_DIRTY)
 				buffers_dirty++;
@@ -306,14 +294,9 @@ pg_buffercache_summary(PG_FUNCTION_ARGS)
 	values[3] = Int32GetDatum(buffers_pinned);
 
 	if (buffers_used != 0)
-	{
-		usagecount_avg = usagecount_avg / buffers_used;
-		values[4] = Float4GetDatum(usagecount_avg);
-	}
+		values[4] = Float8GetDatum((double) usagecount_total / buffers_used);
 	else
-	{
 		nulls[4] = true;
-	}
 
 	/* Build and return the tuple. */
 	tuple = heap_form_tuple(tupledesc, values, nulls);
diff --git a/contrib/pg_buffercache/sql/pg_buffercache.sql b/contrib/pg_buffercache/sql/pg_buffercache.sql
index 897dfbc9245..2e2e0a74517 100644
--- a/contrib/pg_buffercache/sql/pg_buffercache.sql
+++ b/contrib/pg_buffercache/sql/pg_buffercache.sql
@@ -9,3 +9,16 @@ select buffers_used + buffers_unused > 0,
         buffers_dirty <= buffers_used,
         buffers_pinned <= buffers_used
 from pg_buffercache_summary();
+
+-- Check that the functions / views can't be accessed by default. To avoid
+-- having to create a dedicated user, use the pg_database_owner pseudo-role.
+SET ROLE pg_database_owner;
+SELECT * FROM pg_buffercache;
+SELECT * FROM pg_buffercache_pages() AS p (wrong int);
+SELECT * FROM pg_buffercache_summary();
+RESET role;
+
+-- Check that pg_monitor is allowed to query view / function
+SET ROLE pg_monitor;
+SELECT count(*) > 0 FROM pg_buffercache;
+SELECT buffers_used + buffers_unused > 0 FROM pg_buffercache_summary();
diff --git a/doc/src/sgml/pgbuffercache.sgml b/doc/src/sgml/pgbuffercache.sgml
index 6c38ee5d5eb..8f314ee8ff4 100644
--- a/doc/src/sgml/pgbuffercache.sgml
+++ b/doc/src/sgml/pgbuffercache.sgml
@@ -9,27 +9,33 @@
 
  <para>
   The <filename>pg_buffercache</filename> module provides a means for
-  examining what's happening in the shared buffer in real time.
+  examining what's happening in the shared buffer cache in real time.
  </para>
 
  <indexterm>
   <primary>pg_buffercache_pages</primary>
  </indexterm>
 
+ <indexterm>
+  <primary>pg_buffercache_summary</primary>
+ </indexterm>
+
  <para>
-  The module provides C functions <function>pg_buffercache_pages</function>
-  and <function>pg_buffercache_summary</function>.
+  The module provides the <function>pg_buffercache_pages()</function>
+  function, wrapped in the <structname>pg_buffercache</structname> view, and
+  the <function>pg_buffercache_summary()</function> function.
  </para>
 
  <para>
-  The <function>pg_buffercache_pages</function> function
-  returns a set of records, plus a view <structname>pg_buffercache</structname> that wraps the function for
-  convenient use is provided.
+  The <function>pg_buffercache_pages()</function> function returns a set of
+  records, each row describing the state of one shared buffer entry. The
+  <structname>pg_buffercache</structname> view wraps the function for
+  convenient use.
  </para>
 
  <para>
-  The <function>pg_buffercache_summary</function> function returns a table with a single row
-  that contains summarized and aggregated information about shared buffer.
+  The <function>pg_buffercache_summary()</function> function returns a single
+  row summarizing the state of the shared buffer cache.
  </para>
 
  <para>
@@ -174,14 +180,14 @@
  </sect2>
 
  <sect2>
-  <title>The <structname>pg_buffercache_summary</structname> Function</title>
+  <title>The <function>pg_buffercache_summary()</function> Function</title>
 
   <para>
-   The definitions of the columns exposed by the function are shown in <xref linkend="pgbuffercachesummary-columns"/>.
+   The definitions of the columns exposed by the function are shown in <xref linkend="pgbuffercache_summary-columns"/>.
   </para>
 
-  <table id="pgbuffercachesummary-columns">
-   <title><structname>pg_buffercachesummary</structname> Columns</title>
+  <table id="pgbuffercache_summary-columns">
+   <title><function>pg_buffercache_summary()</function> Output Columns</title>
    <tgroup cols="1">
     <thead>
      <row>
@@ -200,7 +206,7 @@
        <structfield>buffers_used</structfield> <type>int4</type>
       </para>
       <para>
-       Number of shared buffers currently being used
+       Number of unused shared buffers
       </para></entry>
      </row>
 
@@ -209,7 +215,7 @@
        <structfield>buffers_unused</structfield> <type>int4</type>
       </para>
       <para>
-        Number of shared buffers that not currently being used
+       Number of unused shared buffers
       </para></entry>
      </row>
 
@@ -227,13 +233,13 @@
        <structfield>buffers_pinned</structfield> <type>int4</type>
       </para>
       <para>
-       Number of shared buffers that has a pinned backend
+       Number of pinned shared buffers
       </para></entry>
      </row>
 
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
-       <structfield>usagecount_avg</structfield> <type>float</type>
+       <structfield>usagecount_avg</structfield> <type>float8</type>
       </para>
       <para>
        Average usagecount of used shared buffers
@@ -244,17 +250,18 @@
   </table>
 
   <para>
-   There is a single row to show summarized information of all shared buffers.
-   <function>pg_buffercache_summary</function> is not interested
-   in the state of each shared buffer, only shows aggregated information.
+   The <function>pg_buffercache_summary()</function> function returns a
+   single row summarizing the state of all shared buffers. Similar and more
+   detailed information is provided by the
+   <structname>pg_buffercache</structname> view, but
+   <function>pg_buffercache_summary()</function> is significantly cheaper.
   </para>
 
   <para>
-   The <function>pg_buffercache_summary</function> doesn't provide a result
-   that is consistent across all buffers. This is intentional. The purpose
-   of this function is to provide a general idea about the state of shared
-   buffers as fast as possible. Additionally, <function>pg_buffercache_summary</function>
-   allocates much less memory.
+   Like the <structname>pg_buffercache</structname> view,
+   <function>pg_buffercache_summary()</function> does not acquire buffer
+   manager locks. Therefore concurrent activity can lead to minor inaccuracies
+   in the result.
   </para>
  </sect2>
 
-- 
2.38.0

