Fix wrong error message from pg_get_tablespace_ddl()

Core Problem

The thread concerns a minor but semantically interesting bug in pg_get_tablespace_ddl_internal() (and by extension its sibling pg_get_role_ddl_internal()), both of which are relatively new DDL-reconstruction helpers in the server. The code comment explicitly states that the required privilege is SELECT on the catalog pg_tablespace, yet when the check fails, the error message is produced against the tablespace object itself:

if (pg_class_aclcheck(TableSpaceRelationId, GetUserId(), ACL_SELECT) != ACLCHECK_OK)
{
    ReleaseSysCache(tuple);
    aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_TABLESPACE, spcname);
}

Passing OBJECT_TABLESPACE with spcname causes aclcheck_error() to emit "permission denied for tablespace ts1", which is misleading: granting privileges on ts1 would not resolve the problem. The underlying privilege being tested is ACL_SELECT against the pg_tablespace catalog relation (via pg_class_aclcheck), not any tablespace-level ACL. So the error points the user at the wrong object.

This is reproducible by revoking SELECT on pg_tablespace from a role and invoking pg_get_tablespace_ddl().

Proposed Solutions and Design Tension

Three distinct philosophies emerged:

1. Literal fix (Chao Li's v1): Change the aclcheck_error call so it reports the failure against pg_tablespace as a table (OBJECT_TABLE). Jim Jones refined this by suggesting get_rel_name(TableSpaceRelationId) instead of hardcoding the catalog name — mirroring the style already used by pg_get_database_ddl_internal() with get_database_name(). Jim also flagged that pg_get_role_ddl_internal() has the analogous bug against pg_authid and should be fixed consistently.

2. Keep the user-facing message (Andrew Dunstan's objection): Andrew argued the current message is arguably correct from the user's perspective. The user asked for DDL of a named object; telling them they lack privilege on that named object is more meaningful than redirecting them to an internal catalog they didn't explicitly query. Exposing the catalog-level check leaks an implementation detail.

3. Rethink the permission model entirely (Álvaro Herrera): Álvaro raised the deeper architectural question — should catalog SELECT even be the gating privilege? The original design intent (in his mind) was that a user with an appropriate grant on the tablespace should be able to reconstruct its DDL even without catalog SELECT. The current implementation conflates two orthogonal access channels. He also raised information-disclosure nuance: should ACL_CREATE on a tablespace reveal spcoptions? Should a database owner whose default tablespace is ts1 see its DDL? Perhaps the location should be redacted for non-superusers while other attributes are shown. This reframes the issue from "wrong error message" to "the privilege model for DDL reconstruction is underspecified."

Resolution Direction

Chao Li's v2 takes a pragmatic middle path that accommodates Andrew's concern without prejudging Álvaro's deeper question:

• Preserve the user-facing error (permission denied for tablespace "ts1") so users aren't told about objects they didn't ask about.
• Add a HINT that reveals the actionable root cause: Grant SELECT on catalog "pg_tablespace" to read tablespace properties.
• Apply the same treatment to pg_get_role_ddl_internal() for pg_authid.
• Use dynamic lookup (avoiding the hardcoded catalog name per Jim's feedback).
• Defer the privilege-model redesign (Álvaro's concerns) to a separate thread.

This preserves backward compatibility of the primary error message while making the error diagnosable. It also correctly pushes back on Chao's own instinct to treat root-cause disclosure as primary — Andrew's point that users think in terms of the object they requested is convincing.

Technical Insights

Why pg_class_aclcheck on a catalog OID?

pg_get_tablespace_ddl() reconstructs DDL by reading tuples from pg_tablespace. Rather than relying on the visibility the catalog already enforces (system catalogs are generally world-readable by default), the function explicitly gates on ACL_SELECT against the catalog so that an administrator who has revoked SELECT from pg_tablespace can still rely on that revocation being honored by the DDL-reconstruction function. This is defense-in-depth: without the explicit check, a SECURITY DEFINER-like path could potentially bypass the revoke.

The aclcheck_error idiom

aclcheck_error(ACLCHECK_NO_PRIV, objtype, objname) is designed to produce the canonical "permission denied for " message. Its contract is that objtype and objname should describe the object the permission was checked against. The bug is therefore a genuine mis-parameterization: the code passed the object the user asked about, not the object the check was performed on.

Parallel bug in pg_get_role_ddl_internal

The role variant uses a raw ereport rather than aclcheck_error, hand-rolling the "permission denied for role %s" message. This is stylistically inconsistent with the tablespace variant and suggests both functions were written without reference to each other. A consistent fix would route both through aclcheck_error (or both through ereport with HINTs).

Álvaro's broader concern: layered DDL visibility

A mature design would distinguish:

• Existence (is there a tablespace named X?) — arguably public.
• Non-sensitive options (spcoptions) — possibly visible to users with CREATE on it or who depend on it.
• Sensitive details (filesystem location) — superuser-only.

PostgreSQL already does analogous redaction elsewhere (e.g., pg_stat_activity query text redaction for non-privileged users). Applying this to DDL reconstruction would require either per-column ACLs on the catalog (which exist but aren't typically used this way) or function-level filtering of the reconstructed DDL string. This is nontrivial and is correctly split off from the immediate fix.

Participant Weight

• Álvaro Herrera — committer, deep catalog/ACL expertise; his questioning of the privilege model carries design authority and is what pushes the thread from "bug fix" to "is the model even right?"
• Andrew Dunstan — committer; his UX-oriented pushback is decisive in shifting v1 → v2 toward preserving the original message.
• Jim Jones — reviewer with good eye for code-consistency issues; catches both the hardcoding smell and the parallel bug in the role variant.
• Chao Li — patch author; notably self-aware (flags that he was a reviewer of the original patch that introduced the bug) and receptive to redirection.
• Zhenwei Shang — independent reproducer, confirms the bug.

Outstanding Issues

1. Whether HINT-based disclosure of the catalog name is itself an information leak (probably not — catalog names are public knowledge).
2. The broader privilege model for pg_get_*_ddl() functions, deferred to a separate thread.
3. Whether test coverage should be added despite Chao's stated reluctance — given that the error message is now part of the contract (with HINT), a regression test becomes more defensible.