public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: Álvaro Herrera <[email protected]>
To: Andrew Dunstan <[email protected]>
To: Jim Jones <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Fix wrong error message from pg_get_tablespace_ddl()
Date: Sat, 9 May 2026 10:01:08 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[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)
view thread (10+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Fix wrong error message from pg_get_tablespace_ddl()
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox