From: Michael Paquier <michael@paquier.xyz>
To: Joel Jacobson <joel@compiler.org>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pgsql: Add pg_get_acl() to get the ACL for a database object
Date: Fri, 5 Jul 2024 08:18:33 +0900
Message-ID: <ZoctyZzE3UlUSer_@paquier.xyz> (raw)
In-Reply-To: <f2688fb7-add6-43da-b1df-97312a451893@app.fastmail.com>
References: <E1sPHWu-000GPc-0z@gemulon.postgresql.org>
<3408967.1720107891@sss.pgh.pa.us>
<f2688fb7-add6-43da-b1df-97312a451893@app.fastmail.com>
(Moving to -hackers)
On Thu, Jul 04, 2024 at 10:53:49PM +0200, Joel Jacobson wrote:
> On Thu, Jul 4, 2024, at 17:44, Tom Lane wrote:
>> Uh, why is it defined like that rather than allowing a subobject?
>> This definition is unable to fetch column-specific ACLs.
Yes, I was wondering about that as well yesterday when looking at the
patch, and saw that this was already a pretty good cut as it covers
most of the objects types and does 90% of the job, so just moved on.
> I wonder if it would be motivated to provide overloads for this function,
> and perhaps even for pg_get_object_address and pg_identify_object_as_address?
>
> That is, two param versions (class OID and object OID),
> and three param versions that in addition also take subobject ID.
I would still stick to only one function, with arguments coming from
scanning pg_[sh]depend.
> I found some code in aclchk.c on line 4452-4468 that seems useful,
> but not sure. Maybe there is some other existing code that is better
> as an inspiration?
My first feelings about that was that subobjids are only used for
pg_attribute, so if we use them as a base, it would look like:
- Adding a new AttrNumber in ObjectProperty to track to which column
the subobjid should apply and its catcache number (ATTNUM for the
attribute number for fast lookup).
- Extend get_catalog_object_by_oid() with more data: the attribute
column for the subobjid scan stored in ObjectProperty, the subobjid
value given by the caller. If get_catalog_object_by_oid()'s caller
defines a subobjid, use get_object_catcache_oid() on it over the main
OID one. If it cannot be found in the cache, use a scan key based on
the class OID and the subobjid.
> I guess we need to handle the RelationRelationId separately,
> and handle all other classes using the current code in pg_get_acl()?
But most of that simply blows away because we track the dependency to
the attribute ACLs in pg_shdepend based on pg_class, not pg_attribute.
get_attname() itself relies on ATTNUM, so perhaps that's OK to just
add an exception based on RelationRelationId in the code path of
pg_get_acl(). That makes me a bit uncomfortable to derive more from
the other routines for the object descriptions, but we've been living
with that in getObjectDescription() for years now, as well.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
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: pgsql-committers@postgresql.org
Cc: michael@paquier.xyz, joel@compiler.org, tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
Subject: Re: pgsql: Add pg_get_acl() to get the ACL for a database object
In-Reply-To: <ZoctyZzE3UlUSer_@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox