Subject: Fix wrong error message from pg_get_tablespace_ddl()

From: Chao Li <[email-masked]>
Date: Fri, 8 May 2026 16:14:40 +0800

Hi,

I started testing pg_get_tablespace_ddl(). While tracing pg_get_tablespace_ddl_internal(), I noticed that this error report must be wrong:
```
	/* User must have SELECT privilege on pg_tablespace. */
	if (pg_class_aclcheck(TableSpaceRelationId, GetUserId(), ACL_SELECT) != ACLCHECK_OK)
	{
		ReleaseSysCache(tuple);
		aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_TABLESPACE, spcname);
	}
```

The comment clearly says that SELECT privilege on pg_tablespace is required, but the error is reported against the target tablespace instead.

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
```

Oops, I was one of the reviewers of the original patch. Sorry for not finding this during review.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

---

From: Zhenwei Shang <[email-masked]>
Date: Fri, 8 May 2026 17:16:21 +0800

Chao Li <[email-masked]> 于2026年5月8日周五 16:15写道：

> Hi,
>
> I started testing pg_get_tablespace_ddl(). While tracing
> pg_get_tablespace_ddl_internal(), I noticed that this error report must be
> wrong:
> ```
> The comment clearly says that SELECT privilege on pg_tablespace is
> required, but the error is reported against the target tablespace instead.
>
> 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:
> ```
Thanks for the patch. I can reproduce the problem and the fix looks correct
to me.

Regards,
Zhenwei Shang

---

From: Jim Jones <[email-masked]>
Date: Fri, 8 May 2026 11:20:20 +0200

Hi Chao

> 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

---

From: Andrew Dunstan <[email-masked]>
Date: Fri, 8 May 2026 08:07:38 -0400

On 2026-05-08 Fr 5:20 AM, Jim Jones wrote:
>> 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:
>> ```
> If we do this change, we should also address pg_authid to keep the 
> code consistent:
>

I'm not 100% convinced these are in fact wrong. The user asks for the 
DDL for a named role or tablespace and we tell them that they don't have 
the privilege to get the information for that object. If we tell them 
that the problem is that they don't have privilege on the underlying 
catalog table, they might think "Well, I didn't ask for that".

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

---

From: Jim Jones <[email-masked]>
Date: Fri, 8 May 2026 14:22:00 +0200

> catalog table, they might think "Well, I didn't ask for that".

I honestly don't have a strong opinion either way.

It depends on what we expect from the error message. If its purpose is 
simply to tell the user "you can't access this object," the current 
message is totally fine. If, however, the goal is to show the error's 
root cause, it could be a bit misleading.

Best, Jim

---

From: Álvaro Herrera <[email-masked]>
Date: Fri, 8 May 2026 19:20:04 +0200

On 2026-May-08, Jim Jones wrote:

> simply to tell the user "you can't access this object," the current message
> is totally fine. If, however, the goal is to show the error's root cause, it
> could be a bit misleading.

Hmm, the idea in my mind was that if SELECT from the catalog is
revoked, but the user does have a grant on the tablespace that lets them
read the DDL, then they should be able to obtain the CREATE statement
for it even though they cannot read the properties from the catalog
directly.  The current coding does not seem to do that, but instead
it refuses to produce the DDL.  Is this really what we want?

Although tablespaces may be special in that only superusers can "own"
them anyway.

TBH I'm undecided about how this should work.  If somebody has
ACL_CREATE on a certain tablespace, should she be able to know what the
spcoptions are, for instance?  What about a database owner whose default
tablespace is that one?  Maybe we'd hide the location unless superuser,
and show the rest ...?

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"This is a foot just waiting to be shot"                (Andrew Dunstan)

---

From: Chao Li <[email-masked]>
Date: Sat, 9 May 2026 10:01:08 +0800

> On May 9, 2026, at 01:20, Álvaro Herrera <[email-masked]> wrote:
> 

Thank you Jim, Andrew, and Álvaro for your feedback.

From Andrew’s comment, I think I was too much driven by the root cause of the problem. From a user’s perspective, if they are trying to view the DDL of "ts1", but the command fails with an error against "pg_tablespace", that could be confusing. So, how about keeping the original error message and adding a hint about how to resolve the error? Otherwise, the user might be misled into granting privileges on "ts1" itself, which would not help resolve the problem. For example:

```
ERROR: permission denied for tablespace ts1
HINT: Grant SELECT on catalog pg_tablespace to read tablespace properties.
```

Álvaro seems to bring the question to a deeper level, and I feel that might be worth a dedicated discussion. For example, I am not sure ACL_CREATE on the tablespace is enough to imply visibility of the tablespace DDL. My understanding is that CREATE on a tablespace allows the user to create objects within that tablespace, but it does not necessarily mean the user is allowed to inspect the definition of the tablespace itself.

How about keeping the scope of this patch narrow, as only adding a hint to guide users on how to fix the error if they really need to view the DDL of the tablespace? I will start a separate thread for the discussion of the access-checking model.

The attached v2 keeps the original error message and adds a hint. I took Jim’s comment about avoiding hardcoding "pg_tablespace”. And I also added a hint in pg_get_role_ddl_internal. With v2, the messages are like:
```
evantest=> select * from pg_get_tablespace_ddl('ts1');
ERROR:  permission denied for tablespace "ts1"
HINT:  Grant SELECT on catalog "pg_tablespace" to read tablespace properties.

evantest=> select * from pg_get_role_ddl('r1');
ERROR:  permission denied for role "r1"
HINT:  Grant SELECT on catalog "pg_authid" to read role properties.
```

Best regards,

---

From: "David G. Johnston" <[email-masked]>
Date: Fri, 8 May 2026 19:52:25 -0700

On Friday, May 8, 2026, Chao Li <[email-masked]> wrote:

> message
> From Andrew’s comment, I think I was too much driven by the root cause of
> the problem. From a user’s perspective, if they are trying to view the DDL
> of "ts1", but the command fails with an error against "pg_tablespace", that
> could be confusing. So, how about keeping the original error message and
> adding a hint about how to resolve the error? Otherwise, the user might be
> misled into granting privileges on "ts1" itself, which would not help
> resolve the problem. For example:
>
> ```
> Álvaro seems to bring the question to a deeper level, and I feel that
> might be worth a dedicated discussion. For example, I am not sure
> ACL_CREATE on the tablespace is enough to imply visibility of the
> tablespace DDL. My understanding is that CREATE on a tablespace allows the
> user to create objects within that tablespace, but it does not necessarily
> mean the user is allowed to inspect the definition of the tablespace itself.
>

The system is designed and built with the assumption that knowledge of
catalog contents are not private (aside from a few security-related cases).
I really don’t see the benefit to jumping through hoops making this feature
work in a world where that isn’t true.  If you cannot read the catalog in
question your superuser did something outside of our design and us choosing
to refuse to produce any DDL requiring the contents of that catalog is
reasonable.  I’d draw the line that if any part of the DDL we would produce
is restricted the entire production is halted, not that we will provide a
best effort result.

IOW, parity with pg_dump seems reasonable.

Reporting which catalogs are restricted is a good message to send.

I’m fine gating the object-based output behind an RLS policies on catalogs
feature so we can at least let people leave select in place and restrict
output to owners/admins of the objects in question.

David J.

---

From: Jim Jones <[email-masked]>
Date: Sun, 10 May 2026 13:10:32 +0200

Hi Chao

> Álvaro seems to bring the question to a deeper level, and I feel that might be worth a dedicated discussion. For example, I am not sure ACL_CREATE on the tablespace is enough to imply visibility of…

Yeah, this is a good point. I don't have a strong opinion about it, but 
I'd be inclined to simply deny access to the DDL if the user does not 
have enough privileges -- at least I wouldn't mind seeing an error 
message in my logs :)

> HINT:  Grant SELECT on catalog "pg_tablespace" to read tablespace properties.

I'm not sure that telling unprivileged users to grant themselves access 
to pg_tablespace is an improvement -- IMO, a HINT here is supposed to be 
actionable. Perhaps a DETAIL would be a better fit, e.g. "DETAIL:  The 
function requires SELECT privilege on catalog "pg_tablespace"."

On top of that, I'm also not sure that replacing the aclcheck_error with 
an ereport just for the hint/detail is an option, since aclcheck_error 
is supposed to provide "Standardized reporting of aclcheck permissions 
failures." (from the aclcheck_error header comment)

Thanks!

Best, Jim

---

From: Andrew Dunstan <[email-masked]>
Date: Sun, 10 May 2026 08:53:17 -0400

On 2026-05-10 Su 7:10 AM, Jim Jones wrote:
>> tablespace DDL. My understanding is that CREATE on a tablespace 
>> allows the user to create objects within that tablespace, but it does 
>> not necessarily mean the user is allowed to inspect the definition of 
>> the tablespace itself.
>
>
> Yeah, this is a good point. I don't have a strong opinion about it, 
> but I'd be inclined to simply deny access to the DDL if the user does 
> not have enough privileges -- at least I wouldn't mind seeing an error 
> message in my logs :)
>
>
>> How about keeping the scope of this patch narrow, as only adding a 
>> hint to guide users on how to fix the error if they really need to 
>> view the DDL of the tablespace? I will start a separate thread for 
>> the discussion of the access-checking model.
>>
>> The attached v2 keeps the original error message and adds a hint. I 
>> took Jim’s comment about avoiding hardcoding "pg_tablespace”. And I 
>> also added a hint in pg_get_role_ddl_internal. With v2, the messages 
>> are like:
>> ```
> I'm not sure that telling unprivileged users to grant themselves 
> access to pg_tablespace is an improvement -- IMO, a HINT here is 
> supposed to be actionable. Perhaps a DETAIL would be a better fit, 
> e.g. "DETAIL:  The function requires SELECT privilege on catalog 
> "pg_tablespace"."
>
> On top of that, I'm also not sure that replacing the aclcheck_error 
> with an ereport just for the hint/detail is an option, since 
> aclcheck_error is supposed to provide "Standardized reporting of 
> aclcheck permissions failures." (from the aclcheck_error header comment)
>
>

I keep coming back to this point: if the user can access pg_tablespace 
they can see the information anyway. This is an informational function, 
and there is no implied guarantee that the user is going to be able to 
run the supplied DDL. I don't think there's anything to do here.

cheers

andrew