Horton

Thread

  1. Fix wrong error message from pg_get_tablespace_ddl()

    Chao Li <li.evan.chao@gmail.com> — 2026-05-08T08:14:40Z

    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/

  2. Re: Fix wrong error message from pg_get_tablespace_ddl()

    Zhenwei Shang <a934172442@gmail.com> — 2026-05-08T09:16:21Z

    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

  3. Re: Fix wrong error message from pg_get_tablespace_ddl()

    Jim Jones <jim.jones@uni-muenster.de> — 2026-05-08T09:20:20Z

    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

  4. Re: Fix wrong error message from pg_get_tablespace_ddl()

    Andrew Dunstan <andrew@dunslane.net> — 2026-05-08T12:07:38Z

    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

  5. Re: Fix wrong error message from pg_get_tablespace_ddl()

    Jim Jones <jim.jones@uni-muenster.de> — 2026-05-08T12:22:00Z

    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

  6. Re: Fix wrong error message from pg_get_tablespace_ddl()

    Álvaro Herrera <alvherre@kurilemu.de> — 2026-05-08T17:20:04Z

    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)

  7. Re: Fix wrong error message from pg_get_tablespace_ddl()

    Chao Li <li.evan.chao@gmail.com> — 2026-05-09T02:01:08Z

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

  8. Re: Fix wrong error message from pg_get_tablespace_ddl()

    David G. Johnston <david.g.johnston@gmail.com> — 2026-05-09T02:52:25Z

    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.

  9. Re: Fix wrong error message from pg_get_tablespace_ddl()

    Jim Jones <jim.jones@uni-muenster.de> — 2026-05-10T11:10:32Z

    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

  10. Re: Fix wrong error message from pg_get_tablespace_ddl()

    Andrew Dunstan <andrew@dunslane.net> — 2026-05-10T12:53:17Z

    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