Date: Thu, 10 Oct 2024 18:28:37 +0200
From: Erik Wienhold <ewie@ewie.name>
To: Dominique Devienne <ddevienne@gmail.com>
Cc: pgsql-general@lists.postgresql.org
Subject: Re: Tablespace ACLs
Message-ID: <10f89717-cd7b-41a8-bde5-c0624b04f039@ewie.name>
References: <CAFCRh--u2GEZ15=WJBRmMwPdKJ3r=_XSA3rw8sjWJZ6Nc2NDYw@mail.gmail.com>
 <711b77fc-f9e5-4637-8a2c-9cc1a3b3f4da@ewie.name>
 <833a8d75-09d7-4691-85bc-1ae0d16097e2@ewie.name>
 <04e246e5-6808-4050-871d-07efc1cea715@ewie.name>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="wm2mka4nmlq6akvv"
Content-Disposition: inline
In-Reply-To: <04e246e5-6808-4050-871d-07efc1cea715@ewie.name>
Archived-At: <https://www.postgresql.org/message-id/10f89717-cd7b-41a8-bde5-c0624b04f039%40ewie.name>
Precedence: bulk


--wm2mka4nmlq6akvv
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

I wrote:
> On second thought, just guarding the pg_tablespace_size() call and
> reporting an unknown size when permissions are insufficient is better,
> so that \db+ still lists all tablespaces like \db does.

And here's a patch that does that.  It also adds a few words to the docs
to mention that the tablespace size is reported as NULL if the user
lacks the necessary access privileges.

I did not add a test for \db+ because the output is not stable due to
varying tablespace OIDs and owner name.

-- 
Erik

--wm2mka4nmlq6akvv
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v1-0001-psql-call-pg_tablespace_size-if-user-has-access-p.patch"

From 00fbfc4025ec8959d5ef29b4912a1a484650ccc9 Mon Sep 17 00:00:00 2001
From: Erik Wienhold <ewie@ewie.name>
Date: Thu, 10 Oct 2024 18:02:00 +0200
Subject: [PATCH v1] psql: call pg_tablespace_size if user has access privilege

---
 doc/src/sgml/ref/psql-ref.sgml |  4 ++--
 src/bin/psql/describe.c        | 25 +++++++++++++++++++++----
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index b825ca96a2..3c8e614a82 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1456,8 +1456,8 @@ SELECT $1 \parse stmt1
         class="parameter">pattern</replaceable>
         is specified, only tablespaces whose names match the pattern are shown.
         If <literal>+</literal> is appended to the command name, each tablespace
-        is listed with its associated options, on-disk size, permissions and
-        description.
+        is listed with its associated options, on-disk size (NULL if user lacks
+        access privileges), permissions and description.
         </para>
         </listitem>
       </varlistentry>
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 6a36c91083..6b6c51ff0f 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -223,7 +223,7 @@ describeTablespaces(const char *pattern, bool verbose)
 	printfPQExpBuffer(&buf,
 					  "SELECT spcname AS \"%s\",\n"
 					  "  pg_catalog.pg_get_userbyid(spcowner) AS \"%s\",\n"
-					  "  pg_catalog.pg_tablespace_location(oid) AS \"%s\"",
+					  "  pg_catalog.pg_tablespace_location(t.oid) AS \"%s\"",
 					  gettext_noop("Name"),
 					  gettext_noop("Owner"),
 					  gettext_noop("Location"));
@@ -232,17 +232,34 @@ describeTablespaces(const char *pattern, bool verbose)
 	{
 		appendPQExpBufferStr(&buf, ",\n  ");
 		printACLColumn(&buf, "spcacl");
+		/*
+		 * Calling pg_tablespace_size(oid) requires certain privileges or the
+		 * it must be the database's default tablespace.  Guard that call and
+		 * report an unknown size by returning NULL if the current role lacks
+		 * the necessary privileges.
+		 */
 		appendPQExpBuffer(&buf,
 						  ",\n  spcoptions AS \"%s\""
-						  ",\n  pg_catalog.pg_size_pretty(pg_catalog.pg_tablespace_size(oid)) AS \"%s\""
-						  ",\n  pg_catalog.shobj_description(oid, 'pg_tablespace') AS \"%s\"",
+						  ",\n  CASE WHEN pg_catalog.has_tablespace_privilege(t.oid, 'CREATE')\n"
+						  "            OR pg_catalog.pg_has_role('pg_read_all_stats', 'USAGE')\n"
+						  "            OR dattablespace IS NOT NULL\n"
+						  "       THEN pg_catalog.pg_size_pretty(pg_catalog.pg_tablespace_size(t.oid))\n"
+						  "       ELSE null\n"
+						  "  END AS \"%s\""
+						  ",\n  pg_catalog.shobj_description(t.oid, 'pg_tablespace') AS \"%s\"",
 						  gettext_noop("Options"),
 						  gettext_noop("Size"),
 						  gettext_noop("Description"));
 	}
 
 	appendPQExpBufferStr(&buf,
-						 "\nFROM pg_catalog.pg_tablespace\n");
+						 "\nFROM pg_catalog.pg_tablespace t\n");
+
+	if (verbose)
+		appendPQExpBufferStr(&buf,
+							 "     LEFT JOIN pg_catalog.pg_database"
+							 " ON datname = pg_catalog.current_database()"
+							 " AND dattablespace = t.oid\n");
 
 	if (!validateSQLNamePattern(&buf, pattern, false, false,
 								NULL, "spcname", NULL,
-- 
2.47.0


--wm2mka4nmlq6akvv--