Re: pgsql: Add pg_get_acl() to get the ACL for a database object

(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