Fix wrong error message from pg_get_tablespace_ddl()

Chao Li <li.evan.chao@gmail.com> 于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:
```
/* 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/

Thanks for the patch. I can reproduce the problem and the fix looks correct
to me.

Regards,
Zhenwei Shang

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

On 2026-05-08 Fr 5:20 AM, Jim Jones wrote:

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));

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

On 08/05/2026 14:07, Andrew Dunstan wrote:

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".

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

On 2026-May-08, Jim Jones wrote:

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.

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)

On May 9, 2026, at 01:20, Álvaro Herrera <alvherre@kurilemu.de> wrote:

On 2026-May-08, Jim Jones wrote:

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.

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)

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,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Friday, May 8, 2026, Chao Li <li.evan.chao@gmail.com> wrote:

On May 9, 2026, at 01:20, Álvaro Herrera <alvherre@kurilemu.de> wrote:

On 2026-May-08, Jim Jones wrote:

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.

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?

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.

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.

Hi Chao

On 09/05/2026 04:01, Chao Li wrote:

Á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.

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

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

On 2026-05-10 Su 7:10 AM, Jim Jones wrote:

Hi Chao

On 09/05/2026 04:01, Chao Li wrote:

Á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.

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

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

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