public inbox for [email protected]
help / color / mirror / Atom feedFix wrong error message from pg_get_tablespace_ddl()
10+ messages / 6 participants
[nested] [flat]
* Fix wrong error message from pg_get_tablespace_ddl()
@ 2026-05-08 08:14 Chao Li <[email protected]>
2026-05-08 09:16 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Zhenwei Shang <[email protected]>
2026-05-08 09:20 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Jim Jones <[email protected]>
0 siblings, 2 replies; 10+ messages in thread
From: Chao Li @ 2026-05-08 08:14 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>
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/
Attachments:
[application/octet-stream] v1-0001-Fix-wrong-error-message-from-pg_get_tablespace_dd.patch (1.3K, 2-v1-0001-Fix-wrong-error-message-from-pg_get_tablespace_dd.patch)
download | inline diff:
From 0ffd155eca7691b16fe799f43a776a7dc59f3b7c Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Fri, 8 May 2026 15:53:23 +0800
Subject: [PATCH v1] Fix wrong error message from pg_get_tablespace_ddl()
pg_get_tablespace_ddl_internal() checks ACL_SELECT on the pg_tablespace
catalog relation before reading the tablespace tuple. However, when that
check failed, it reported the error as permission denied for the specific
tablespace object.
Report the failure as permission denied for table pg_tablespace instead,
matching the object on which the permission check was actually performed.
Author: Chao Li <[email protected]>
---
src/backend/utils/adt/ddlutils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/ddlutils.c b/src/backend/utils/adt/ddlutils.c
index d6f55c48f37..11bf5ee5836 100644
--- a/src/backend/utils/adt/ddlutils.c
+++ b/src/backend/utils/adt/ddlutils.c
@@ -681,7 +681,7 @@ pg_get_tablespace_ddl_internal(Oid tsid, bool pretty, bool no_owner)
if (pg_class_aclcheck(TableSpaceRelationId, GetUserId(), ACL_SELECT) != ACLCHECK_OK)
{
ReleaseSysCache(tuple);
- aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_TABLESPACE, spcname);
+ aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_TABLE, "pg_tablespace");
}
/*
--
2.50.1 (Apple Git-155)
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix wrong error message from pg_get_tablespace_ddl()
2026-05-08 08:14 Fix wrong error message from pg_get_tablespace_ddl() Chao Li <[email protected]>
@ 2026-05-08 09:16 ` Zhenwei Shang <[email protected]>
1 sibling, 0 replies; 10+ messages in thread
From: Zhenwei Shang @ 2026-05-08 09:16 UTC (permalink / raw)
To: Chao Li <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
Chao Li <[email protected]> 于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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix wrong error message from pg_get_tablespace_ddl()
2026-05-08 08:14 Fix wrong error message from pg_get_tablespace_ddl() Chao Li <[email protected]>
@ 2026-05-08 09:20 ` Jim Jones <[email protected]>
2026-05-08 12:07 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Andrew Dunstan <[email protected]>
1 sibling, 1 reply; 10+ messages in thread
From: Jim Jones @ 2026-05-08 09:20 UTC (permalink / raw)
To: Chao Li <[email protected]>; PostgreSQL Hackers <[email protected]>
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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix wrong error message from pg_get_tablespace_ddl()
2026-05-08 08:14 Fix wrong error message from pg_get_tablespace_ddl() Chao Li <[email protected]>
2026-05-08 09:20 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Jim Jones <[email protected]>
@ 2026-05-08 12:07 ` Andrew Dunstan <[email protected]>
2026-05-08 12:22 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Jim Jones <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Dunstan @ 2026-05-08 12:07 UTC (permalink / raw)
To: Jim Jones <[email protected]>; Chao Li <[email protected]>; PostgreSQL Hackers <[email protected]>
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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix wrong error message from pg_get_tablespace_ddl()
2026-05-08 08:14 Fix wrong error message from pg_get_tablespace_ddl() Chao Li <[email protected]>
2026-05-08 09:20 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Jim Jones <[email protected]>
2026-05-08 12:07 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Andrew Dunstan <[email protected]>
@ 2026-05-08 12:22 ` Jim Jones <[email protected]>
2026-05-08 17:20 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Álvaro Herrera <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Jim Jones @ 2026-05-08 12:22 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; Chao Li <[email protected]>; PostgreSQL Hackers <[email protected]>
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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix wrong error message from pg_get_tablespace_ddl()
2026-05-08 08:14 Fix wrong error message from pg_get_tablespace_ddl() Chao Li <[email protected]>
2026-05-08 09:20 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Jim Jones <[email protected]>
2026-05-08 12:07 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Andrew Dunstan <[email protected]>
2026-05-08 12:22 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Jim Jones <[email protected]>
@ 2026-05-08 17:20 ` Álvaro Herrera <[email protected]>
2026-05-09 02:01 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Chao Li <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Álvaro Herrera @ 2026-05-08 17:20 UTC (permalink / raw)
To: Jim Jones <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Chao Li <[email protected]>; PostgreSQL Hackers <[email protected]>
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)
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix wrong error message from pg_get_tablespace_ddl()
2026-05-08 08:14 Fix wrong error message from pg_get_tablespace_ddl() Chao Li <[email protected]>
2026-05-08 09:20 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Jim Jones <[email protected]>
2026-05-08 12:07 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Andrew Dunstan <[email protected]>
2026-05-08 12:22 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Jim Jones <[email protected]>
2026-05-08 17:20 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Álvaro Herrera <[email protected]>
@ 2026-05-09 02:01 ` Chao Li <[email protected]>
2026-05-09 02:52 ` Re: Fix wrong error message from pg_get_tablespace_ddl() David G. Johnston <[email protected]>
2026-05-10 11:10 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Jim Jones <[email protected]>
0 siblings, 2 replies; 10+ messages in thread
From: Chao Li @ 2026-05-09 02:01 UTC (permalink / raw)
To: Álvaro Herrera <[email protected]>; Andrew Dunstan <[email protected]>; Jim Jones <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
> On May 9, 2026, at 01:20, Álvaro Herrera <[email protected]> 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/
Attachments:
[application/octet-stream] v2-0001-ddlutils-add-hints-for-catalog-privilege-failures.patch (2.5K, 2-v2-0001-ddlutils-add-hints-for-catalog-privilege-failures.patch)
download | inline diff:
From 1515cc70ba858c8991dbf2fca9b3b835bff25d78 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Fri, 8 May 2026 15:53:23 +0800
Subject: [PATCH v2] ddlutils: add hints for catalog privilege failures
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
pg_get_role_ddl_internal() and pg_get_tablespace_ddl_internal() require
SELECT privilege on their underlying catalogs before reading object
properties. When this privilege check fails, the error message reports
the user-facing object, such as the role or tablespace whose DDL was
requested, but it does not tell the user which privilege is actually
missing.
Add errhint() messages to explain that SELECT on the relevant catalog is
needed to read the object properties. This keeps the primary error
message focused on the requested object, while giving users a concrete
way to resolve the failure.
Author: Chao Li <[email protected]>
Reviewed-by: Jim Jones <[email protected]>
Reviewed-by: Andrew Dunstan <[email protected]>
Reviewed-by: Álvaro Herrera <[email protected]>
Reviewed-by: Zhenwei Shang <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/backend/utils/adt/ddlutils.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/adt/ddlutils.c b/src/backend/utils/adt/ddlutils.c
index d6f55c48f37..db72854b1c5 100644
--- a/src/backend/utils/adt/ddlutils.c
+++ b/src/backend/utils/adt/ddlutils.c
@@ -342,8 +342,10 @@ pg_get_role_ddl_internal(Oid roleid, bool pretty, bool memberships)
{
ReleaseSysCache(tuple);
ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("permission denied for role %s", rolname)));
+ errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied for role \"%s\"", rolname),
+ errhint("Grant SELECT on catalog \"%s\" to read role properties.",
+ get_rel_name(AuthIdRelationId)));
}
/*
@@ -681,7 +683,12 @@ pg_get_tablespace_ddl_internal(Oid tsid, bool pretty, bool no_owner)
if (pg_class_aclcheck(TableSpaceRelationId, GetUserId(), ACL_SELECT) != ACLCHECK_OK)
{
ReleaseSysCache(tuple);
- aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_TABLESPACE, spcname);
+ ereport(ERROR,
+ errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied for tablespace \"%s\"",
+ spcname),
+ errhint("Grant SELECT on catalog \"%s\" to read tablespace properties.",
+ get_rel_name(TableSpaceRelationId)));
}
/*
--
2.50.1 (Apple Git-155)
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix wrong error message from pg_get_tablespace_ddl()
2026-05-08 08:14 Fix wrong error message from pg_get_tablespace_ddl() Chao Li <[email protected]>
2026-05-08 09:20 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Jim Jones <[email protected]>
2026-05-08 12:07 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Andrew Dunstan <[email protected]>
2026-05-08 12:22 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Jim Jones <[email protected]>
2026-05-08 17:20 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Álvaro Herrera <[email protected]>
2026-05-09 02:01 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Chao Li <[email protected]>
@ 2026-05-09 02:52 ` David G. Johnston <[email protected]>
1 sibling, 0 replies; 10+ messages in thread
From: David G. Johnston @ 2026-05-09 02:52 UTC (permalink / raw)
To: Chao Li <[email protected]>; +Cc: Álvaro Herrera <[email protected]>; Andrew Dunstan <[email protected]>; Jim Jones <[email protected]>; PostgreSQL Hackers <[email protected]>
On Friday, May 8, 2026, Chao Li <[email protected]> wrote:
>
>
> > On May 9, 2026, at 01:20, Álvaro Herrera <[email protected]> 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.
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix wrong error message from pg_get_tablespace_ddl()
2026-05-08 08:14 Fix wrong error message from pg_get_tablespace_ddl() Chao Li <[email protected]>
2026-05-08 09:20 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Jim Jones <[email protected]>
2026-05-08 12:07 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Andrew Dunstan <[email protected]>
2026-05-08 12:22 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Jim Jones <[email protected]>
2026-05-08 17:20 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Álvaro Herrera <[email protected]>
2026-05-09 02:01 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Chao Li <[email protected]>
@ 2026-05-10 11:10 ` Jim Jones <[email protected]>
2026-05-10 12:53 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Andrew Dunstan <[email protected]>
1 sibling, 1 reply; 10+ messages in thread
From: Jim Jones @ 2026-05-10 11:10 UTC (permalink / raw)
To: Chao Li <[email protected]>; Álvaro Herrera <[email protected]>; Andrew Dunstan <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix wrong error message from pg_get_tablespace_ddl()
2026-05-08 08:14 Fix wrong error message from pg_get_tablespace_ddl() Chao Li <[email protected]>
2026-05-08 09:20 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Jim Jones <[email protected]>
2026-05-08 12:07 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Andrew Dunstan <[email protected]>
2026-05-08 12:22 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Jim Jones <[email protected]>
2026-05-08 17:20 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Álvaro Herrera <[email protected]>
2026-05-09 02:01 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Chao Li <[email protected]>
2026-05-10 11:10 ` Re: Fix wrong error message from pg_get_tablespace_ddl() Jim Jones <[email protected]>
@ 2026-05-10 12:53 ` Andrew Dunstan <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Dunstan @ 2026-05-10 12:53 UTC (permalink / raw)
To: Jim Jones <[email protected]>; Chao Li <[email protected]>; Álvaro Herrera <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
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
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2026-05-10 12:53 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-05-08 08:14 Fix wrong error message from pg_get_tablespace_ddl() Chao Li <[email protected]>
2026-05-08 09:16 ` Zhenwei Shang <[email protected]>
2026-05-08 09:20 ` Jim Jones <[email protected]>
2026-05-08 12:07 ` Andrew Dunstan <[email protected]>
2026-05-08 12:22 ` Jim Jones <[email protected]>
2026-05-08 17:20 ` Álvaro Herrera <[email protected]>
2026-05-09 02:01 ` Chao Li <[email protected]>
2026-05-09 02:52 ` David G. Johnston <[email protected]>
2026-05-10 11:10 ` Jim Jones <[email protected]>
2026-05-10 12:53 ` Andrew Dunstan <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox