Message-ID: <9cc5e7a5-1cbc-4e07-ace4-ad04a8e1a6fe@uni-muenster.de>
Date: Fri, 8 May 2026 11:20:20 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Fix wrong error message from pg_get_tablespace_ddl()
To: Chao Li <li.evan.chao@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <53D05145-CE87-424F-A492-BB22A09BBE11@gmail.com>
Content-Language: en-US, de-DE
From: Jim Jones <jim.jones@uni-muenster.de>
In-Reply-To: <53D05145-CE87-424F-A492-BB22A09BBE11@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/9cc5e7a5-1cbc-4e07-ace4-ad04a8e1a6fe%40uni-muenster.de>
Precedence: bulk

Hi Chao

On 08/05/2026 10:14, Chao Li wrote:
> This is easy to reproduce:
> ```
> evantest=# set allow_in_place_tablespaces = true;
> SET
> evantest=# create role r1;
> CREATE ROLE
> evantest=# create tablespace ts1 location '';
> CREATE TABLESPACE
> evantest=# revoke select on pg_tablespace from r1;
> REVOKE
> evantest=# set role r1;
> SET
> evantest=> select * from pg_get_tablespace_ddl('ts1');
> ERROR:  permission denied for tablespace ts1
> ```
> 
> Attached is a simple one-line fix. Attached is a simple one-line fix. I did not add a new test, as we usually try to avoid extending the test time for such a small fix. With the fix, the error message now looks like:
> ```
> evantest=> select * from pg_get_tablespace_ddl('ts1');
> ERROR:  permission denied for table pg_tablespace
> ```

A few comments:

== hardcoded table name ==

Hardcoding "pg_tablespace" looks IMO a bit fragile. What about
get_rel_name(TableSpaceRelationId) instead?

See get_database_name(dbid) in pg_get_database_ddl_internal().

== similar issue in pg_get_role_ddl_internal ==

If we do this change, we should also address pg_authid to keep the code 
consistent:

/* User must have SELECT privilege on pg_authid. */
if (pg_class_aclcheck(AuthIdRelationId, GetUserId(), ACL_SELECT) != 
ACLCHECK_OK)
{
   ReleaseSysCache(tuple);
   ereport(ERROR,
   (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
	errmsg("permission denied for role %s", rolname)));
}

Perhaps something like this instead of the ereport:

aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_TABLE, 
get_rel_name(AuthIdRelationId));

Thanks!

Best, Jim